Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve(Relayer): Don't fill exclusive deposits #2017

Merged
merged 14 commits into from
Feb 7, 2025

Conversation

nicholaspai
Copy link
Member

@nicholaspai nicholaspai commented Jan 26, 2025

SpokePool's today prevent filling deposits if exclusivityDeadline >= currentTime and msg.sender !== exclusiveRelayer

The exclusive deadline is still respected even if the exclusiveRelayer = zero address so the relayer has a bad check in it currently that is causing it to try to fill deposits with an exclusivity deadline > current time but the exclusiveRelayer == 0

This PR should greatly reduce the simulation errors in the relayer

This is the existing contract code
image

Soneium and Ink SpokePool

These two contracts actually allow ignore the exclusivityDeadline upon filling if the exclusiveRelayer == 0 but rather than adding special cases for these chains I propose using this fix unilaterally as it will be forwards compatible with the changes to exclusivity soon to be deployed

SpokePool's today prevent filling deposits if `exclusivityDeadline >=  currentTime and msg.sender !== exclusiveRelayer`

The exclusive deadline is still respected if the exclusiveRelayer = zero address so the relayer has a bad check in it currently that is causing it to try to fill deposits with an exclusivity deadline > current time but the exclusiveRelayer == 0

This PR should greatly reduce the simulation errors in the relayer
@@ -292,11 +291,7 @@ export class Relayer {
return false;
}

if (
deposit.exclusiveRelayer !== ZERO_ADDRESS &&
deposit.exclusivityDeadline > currentTime &&
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think this should be a >= instead of a > because the deposit is exclusive up until the exclusive timestamp exactly

mrice32
mrice32 previously approved these changes Jan 26, 2025
@nicholaspai nicholaspai added the do not merge Don't merge until label is removed label Jan 26, 2025
@nicholaspai
Copy link
Member Author

Let's wait on merging this until all spoke pools are upgraded such that the exclusive relayer being 0 is impossible on the deposit side if an exclusivity deadline is sent and the exclusive relayer being 0 on the fill side is not a special case.

We can live with the extra noise for now

@nicholaspai nicholaspai requested a review from bmzig as a code owner January 27, 2025 15:18
james-a-morris
james-a-morris previously approved these changes Jan 29, 2025
Copy link
Contributor

@james-a-morris james-a-morris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@nicholaspai
Copy link
Member Author

Can merge this after spoke pool upgrade, otherwise we'll ignore filling deposits that have exclusivityDeadline > currentTime even if the exclusiveRelayer is 0x

@nicholaspai nicholaspai dismissed stale reviews from mrice32 and james-a-morris via a8f61e0 February 6, 2025 01:44
@nicholaspai nicholaspai requested a review from mrice32 February 6, 2025 02:14
@nicholaspai nicholaspai changed the base branch from master to contractUpgrade February 6, 2025 22:01
@nicholaspai nicholaspai merged commit 718ea10 into contractUpgrade Feb 7, 2025
4 checks passed
@nicholaspai nicholaspai deleted the exclusivity-deadline-relayer branch February 7, 2025 01:13
bmzig added a commit that referenced this pull request Feb 7, 2025
* feat: support updated spoke pool functions

Signed-off-by: bennett <[email protected]>

* fix

Signed-off-by: bennett <[email protected]>

* missing toBytes32

Signed-off-by: bennett <[email protected]>

* feat: support contract upgrade

Signed-off-by: bennett <[email protected]>

* test

Signed-off-by: bennett <[email protected]>

* Get relayer tests to pass

- skipped all tests that have relayer call fillV3RelayWithUpdatedDeposit and requetV3SlowFill

* add new events

Signed-off-by: bennett <[email protected]>

* fix

Signed-off-by: bennett <[email protected]>

* Fix tests with speedUpDeposit, requestSlowFill, executeSlowRelayLeaf

* undefined

Signed-off-by: bennett <[email protected]>

* monitor tests

Signed-off-by: bennett <[email protected]>

* Partial fix

* fix relayer change

Signed-off-by: bennett <[email protected]>

* fix: add old spoke ABIs

Signed-off-by: Matt Rice <[email protected]>

* fix Dataworker.buildRoots.ts

* Update Dataworker.executeSlowRelay.ts

* Update Dataworker.loadData.prefill.ts

* fix build

Signed-off-by: Matt Rice <[email protected]>

* add new events

Signed-off-by: bennett <[email protected]>

* fix

Signed-off-by: bennett <[email protected]>

* undefined

Signed-off-by: bennett <[email protected]>

* monitor tests

Signed-off-by: bennett <[email protected]>

* Partial fix

* fix relayer change

Signed-off-by: bennett <[email protected]>

* fix Dataworker.buildRoots.ts

* Update Dataworker.executeSlowRelay.ts

* Update Dataworker.loadData.prefill.ts

* Update Dataworker.executeSlowRelay.ts

* Add backwards compatibility

* Add flags to tests

* lint

* fix

* Fix test

* Update Dataworker.executePoolRebalances.ts

* feat: support updated spoke pool functions (#2043)

* feat: support updated spoke pool functions

Signed-off-by: bennett <[email protected]>

* fix

Signed-off-by: bennett <[email protected]>

* missing toBytes32

Signed-off-by: bennett <[email protected]>

* Fix tests with speedUpDeposit, requestSlowFill, executeSlowRelayLeaf

* fix: add old spoke ABIs

Signed-off-by: Matt Rice <[email protected]>

* fix build

Signed-off-by: Matt Rice <[email protected]>

* add new events

Signed-off-by: bennett <[email protected]>

* fix

Signed-off-by: bennett <[email protected]>

* undefined

Signed-off-by: bennett <[email protected]>

* monitor tests

Signed-off-by: bennett <[email protected]>

* Partial fix

* fix relayer change

Signed-off-by: bennett <[email protected]>

* fix Dataworker.buildRoots.ts

* Update Dataworker.executeSlowRelay.ts

* Update Dataworker.loadData.prefill.ts

* Update Dataworker.executeSlowRelay.ts

* Add backwards compatibility

* Add flags to tests

* lint

* fix

* Fix test

* Update Dataworker.executePoolRebalances.ts

---------

Signed-off-by: bennett <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Co-authored-by: nicholaspai <[email protected]>
Co-authored-by: Matt Rice <[email protected]>
Co-authored-by: nicholaspai <[email protected]>

* Update MerkleTreeUtils.ts

* improve(Relayer): Don't fill exclusive deposits (#2017)

* improve(Relayer): Don't fill exclusive deposits

SpokePool's today prevent filling deposits if `exclusivityDeadline >=  currentTime and msg.sender !== exclusiveRelayer`

The exclusive deadline is still respected if the exclusiveRelayer = zero address so the relayer has a bad check in it currently that is causing it to try to fill deposits with an exclusivity deadline > current time but the exclusiveRelayer == 0

This PR should greatly reduce the simulation errors in the relayer

* Update Relayer.ts

* cannot request slow fills in exclusivity window

* Update Relayer.ts

* bump sdk

Signed-off-by: bennett <[email protected]>

---------

Signed-off-by: bennett <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Co-authored-by: nicholaspai <[email protected]>
Co-authored-by: Matt Rice <[email protected]>
Co-authored-by: nicholaspai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Don't merge until label is removed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants