feat: Support pending SpokePool upgrades #611

pxrl · 2025-01-17T21:12:55Z

This is a first pass at establishing concurrent support for the new & legacy events.

This is a first pass at establishing concurrent support for the new & legacy events. There's stilll work to be done to (at least): - Specify how to handle new relayer repayment capabilities. - Specify how to handle pre-fills where the relayer orchestrates the deposit after the fill. There are edge cases to consider.

UMIPs/umip-179.md

pxrl · 2025-01-20T14:38:48Z

UMIPs/umip-179.md

+#### Validating Pre-fills
+For each of the `Deposits` emitted within the target block range where no corresponding `Fill` is identified on the destination chain, identify the valid `Fill` according to the following criteria:
+1. Verify that the destination chain `FillStatus` for the `Deposit` `RelayData` is `Filled` as at the destination chain end block number for the proposal.
+2. Resolve the corresponding `Fill` on the destination chain.
+
+#### Note
+- No specific method is prescribed for resolving the fill on the destination chain. An `eth_getLogs` request can facilitate this, and if required, the target block range could be narrowed by a binary search over the `FillStatus` field. This is left as an implementation decision.


I added this section to describe how to identify pre-fills. This is a fairly abstract description; it arises out of the realisation that an arbitrarily-old fill with a non-deterministic depositId can still be identified via a binary search, which should remove the need to artificially limit the fill->deposit time delta. This should be naively fine OK as long as SpokePool addresses and token mappings don't change. We might consider adding a note to explicitly warn relayers of these risks.

fwiw, the SDK already has an implementation of the necessary logic for a binary search here: https://github.com/across-protocol/sdk/blob/db244fbaec15dc049452cd104ca7bec5856ad3da/src/utils/SpokeUtils.ts#L328 This implementation came out of the resilience work that was done, because RPC provider jank can also cause a deposit without a corresponding fill event.

There is a consideration about the load imposed on proposers and disputers from a binary search, which can only be done per-deposit. I think this is the optimal strategy for now, where only a small number of deposits are expected to fall outside of the proposer/disputer lookback window. The strategy could be improved if the ratio of pre-fills started to become significant. One candidate for optimising this could be to just increase the block range when querying for fills. Since most missing fills are probably only hours old, this strategy could be used quite effectively.

If the load from increasing the getLogs range did ultimately produce a scalability problem then I think we'd ultimately look to bound the fill -> deposit delta by some multiple of the fillDeadline specified in the deposit. But I think that should be reserved for a future UMIP change (and may not even be necessary).

nicholaspai

This is a fantastic start. My biggest questions at the moment are in the new validating fills section

UMIPs/umip-179.md

nicholaspai · 2025-01-21T03:07:09Z

UMIPs/umip-179.md

+- The `FundsDeposited` and `V3FundsDeposited` `outputToken` field is not required to be a known HubPool `l1Token`. In-protocol arbitrary token swaps are technically supported by Across v3.
+- The address identified by `exclusiveRelayer` has exclusive right to complete the relay on the destination chain until `exclusivityDeadline` has elapsed.
+- If `exclusivityDeadline` is set to a past timestamp, any address is eligible to fill the relay.
+- Any deposit that remains unfilled after the specified `fillDeadline` shall be refunded to the `depositor` address via the origin SpokePool in a subsequent settlement bundle.


What about if the deposit is a duplicate relay hash? Which is now possible with non uniqueness of deposit ids

Just to tease this one out into a scenario: unsafeDeposit() is called, producing a relayDataHash collision on the destination SpokePool. The proposer concludes that the deposit was filled by some previous, unrelated fill. The proposer then identifies that fill and credits the relayer with the repayment, meaning the relayer gets paid twice and the depositor can never receive their refund. Is this the same scenario you're thinking of?

In this specific case, I think there is some risk that the depositor (or relayer, or more broadly the "facilitator" of the order flow) takes when they call unsafeDeposit() (this is basically why it's called unsafe). It is however possible for them to check the FillStatus on the destination chain in advance of selecting the salt for their depositId, so they can avoid the scenario where they collide with a very old fill. I think this then reduces the risk to a collision occurring within a much shorter period of time - i.e. seconds or minutes.

This might not be the only scenario, and my reasoning might be flawed on this. This scenario alone also indicates that we need to add some guidance in our docs for how integrators should select their salt when generating pre-fill deposits for depositors to sign. wdyt?

One additional and very simple check that could protect against this - the proposer and disputer could be required to verify both the relayDataHash and a subset of the fields - i.e. to verify that some or all of the following fields are an exact match. That would protect against relayDataHash collisions, and would allow the proposer to refund the deposit on the origin chain:

depositor

recipient

inputToken

inputAmount

outputAmount

fillDeadline

The proposer then identifies that fill and credits the relayer with the repayment, meaning the relayer gets paid twice and the depositor can never receive their refund. Is this the same scenario you're thinking of?

No, in this case the relayer could only fill once because the relay hash is the same for both deposits. The deposit pays twice and only gets filled once. The relayer gets refunded for one their one fill

But overall I think the function is aptly named "unsafe" and depositors who do this should be aware they might lose their deposit.

No, in this case the relayer could only fill once because the relay hash is the same for both deposits. The deposit pays twice and only gets filled once. The relayer gets refunded for one their one fill

If the proposer/disputer resolves solely on RelayData hash then I'd still expect it to match a new deposit (via unsafeDeposit()) with a very old fill that was made for whatever other deposit that collided. So in that case, it'd conclude that the old fill was valid and would then enqueue a relayer refund, and would not refund the depositor.

So the argument is not that the fill would occur twice, but that the proposer/disputer would issue two relayer repayments for the same fill. Does that make sense?

Currently the proposer will not reward a refund for any duplicate expired deposits -- only the first would get refunded: across-protocol/sdk#838

If the relayHash matches, these would have to match, right? Otherwise, we've found a keccak hash collision, no? One important thing to also note is that it's pretty hard to spoof collisions without controlling the calling user's wallet. The depositor and msg.sender (in addition to nonce) must match to create a collision.

Agreed! The risk of a depositor reusing their salt (depositNonce) is much more realistic. It's a lot easier to create collisions now that deposit permits fillDeadline to be in the past.

If I'm understanding correctly, the primary motivation for the double relayer payment is that we don't have a good way to go back and find a deposit from long ago, so we can't distinguish between a prefill and double deposit. But I also think the most likely failure mode is a double transaction submission, where the deposits are very close together, so if we could protect users in that case, I think this sharp edge wouldn't be too painful for users.

Thoughts?

I still feel allergic to the possibility of a double repayment, even if it can be confined to rare cases. It's very counter-intuitive and is likely to be quite tricky to investigate, if it ever happens. I'm much more in favour of finding solutions where we can eliminate the possibility. It's unclear whether we can achieve that given other constraints though.

Looks like this isn't resolved AFAICT. The UMIP suggests that deposits are refunded if not filled, but I don't see a clarification about what "filled" really means here (i.e. a duplicate deposits are allowed and can all be considered filled by the same fill and thus result in loss of funds).

We have a couple of threads adjacent to this - I left a response here. Are you OK with the proposed definition for what a filled relay is?

Just to round this out, I think we settled on the maximally simple definition of a fill - tldr, a deposit is filled when the deposit FillStatus on the destination chain is in state Filled.

UMIPs/umip-179.md

nicholaspai · 2025-01-21T03:12:44Z

UMIPs/umip-179.md

 - The applicable rate model shall be sourced from the AcrossConfigStore contract for the relevant `l1Token`.
 - Deposits where the `originChainId` is a "Lite" chain in the AcrossConfigStore as of the `quoteTimestamp` shall always be repaid on the deposit's origin chain. This means the protocol overrides the relayer's requested `repaymentChainId` with the `originChainId` instead.

+The recipient of relayer repayments shall be the `Fill` `relayer` address. If the `relayer` address is invalid for the respective `repaymentChainId`, the proposer shall:
+- Issue the refund on the `destinationChainId` instead of the `repaymentChainId`, AND


What if the destination chain is a lite chain?

I think it's OK when the destination chain is a lite chain (we permit this currently), but it's not OK when the origin chain is a lite chain (good catch). So the logic needs to be tightened up, such that this has a lower precedence than the lite chain constraints.

@nicholaspai Updated here to be more explicit about the precedence of the applied repaymentChainId and reverting to a fallback repayment address in case of an invalid relayer-specified address for the applied repaymentChainId.

wdyt?

left one comment a95d437#r1923960561

UMIPs/umip-179.md

Co-authored-by: nicholaspai <[email protected]>

UMIPs/umip-179.md

bmzig · 2025-01-21T18:34:20Z

UMIPs/umip-179.md


 ### Computing Deposit Refunds
-For an expired `V3FundsDeposited` event, the depositor refund amount shall be computed as `inputAmount` units of `inputToken`.
+For an expired `Deposit` event, the depositor refund amount shall be computed as `inputAmount` units of `inputToken`.


I think you should also say that if you set the depositor field to an invalid EVM address, (i.e. a bytes32 address), then you won't be refunded if you aren't filled.

@mrice32 @nicholaspai This is something that we can enforce at the contract level, and not enforcing it there leads to this risk that the depositor's funds get locked in the protocol. It costs a bit more gas to do this but gives a lot more safety to the depositor. Given that we already need to modify the SpokePool for the RelayData hash, would we consider also enforcing that depositor is a valid EVM address?

Description updated so that refunds for invalid depositor addresses are discarded: cab40f8

Leaving this open for now, pending confirmation of across-protocol/contracts#874 being merged.

UMIPs/umip-179.md

pxrl · 2025-01-22T15:04:38Z

UMIPs/umip-179.md

 | Name | Type | Description |
 | :--- | :--- | :---------- |
 | relayer | address | The address completing relay on the destination SpokePool. |
 | repaymentChainId | uint256 | The depositId of the corresponding `V3FundsDeposited` event to be updated. |
-| relayExecutionInfo | V3RelayExecutionInfo | The effective `recipient`, `message` and `outputAmount`, as well as the `FillType` performed (FastFill, ReplacedSlowFill, SlowFill). |
+| relayExecutionInfo | V3RelayExecutionEventInfo | The effective `recipient`, `message` and `outputAmount`, as well as the `FillType` performed (FastFill, ReplacedSlowFill, SlowFill). |


This may need an update, subject to across-protocol/contracts#872

…re-slowfill-requests ### Definitions - "pre-fill": any fill that is in a bundle that precedes the bundle containing the matched deposit - "pre-slow-fill-request": a mouthful, and any slow fill request in a bundle that precedes the bundle containing the matched deposit ## Core logic for refunding pre-fills: Evaluate all deposits in the current bundle that were not filled in the current bundle. Figure out whether the deposit was filled, by either: - Finding the fill in the spoke pool client's memory, or - Querying the fill status on-chain If there is a fill for this deposit then we need to issue a refund for this fill because the fill occurred in a previous bundle where it might not have been refunded. We need to use a similar algorithm for creating slow fill leaves for pre-slow-fill-requests ## Avoiding duplicate refunds for pre-fills We don't deterministically know whether this pre-fill was refunded in the prior bundle because we don't know for certain what kind of event search window the proposer of the prior bundle used when constructing the bundle. To illustrate this problem, imagine if a fill and a deposit are sent 10 minutes apart such that the fill is at the end of the current bundle and the deposit is at the beginning of the next. The prior bundle proposer probably would have issued a refund for this fill, but we don't know for certain. So, one way around this non-determinism is to introduce a new rule to the UMIP: ### Do not issue refunds for fills or slow fill leaves where the matched deposit is later than the current bundle block range This rule makes it always apparent which bundle should include a refund or slow fill leaf for a pre-fill or pre-slow-fill-request. This will require a UMIP change to [this PR](UMAprotocol/UMIPs#611) ## TODO I still need to update the `findFillBlock()` function to return a historical fill for a deposit, because we'll need the FilledRelay's `relayerAddress` or `msg.sender` to credit the refund to (the latter in the case where the relayer address isn't valid on the repayment chain).

mrice32

Looks great! A few comments

UMIPs/umip-179.md

mrice32 · 2025-01-23T13:35:53Z

UMIPs/umip-179.md

+- The `FundsDeposited` and `V3FundsDeposited` `outputToken` field is not required to be a known HubPool `l1Token`. In-protocol arbitrary token swaps are technically supported by Across v3.
+- The address identified by `exclusiveRelayer` has exclusive right to complete the relay on the destination chain until `exclusivityDeadline` has elapsed.
+- If `exclusivityDeadline` is set to a past timestamp, any address is eligible to fill the relay.
+- Any deposit that remains unfilled after the specified `fillDeadline` shall be refunded to the `depositor` address via the origin SpokePool in a subsequent settlement bundle.


One additional and very simple check that could protect against this - the proposer and disputer could be required to verify both the relayDataHash and a subset of the fields

If the relayHash matches, these would have to match, right? Otherwise, we've found a keccak hash collision, no? One important thing to also note is that it's pretty hard to spoof collisions without controlling the calling user's wallet. The depositor and msg.sender (in addition to nonce) must match to create a collision.

The proposer then identifies that fill and credits the relayer with the repayment, meaning the relayer gets paid twice and the depositor can never receive their refund. Is this the same scenario you're thinking of?

Do we want this behavior to be universal? Or if the deposits are within some range of one another, we do a refund (since the duplicate is detectable in that case)?

If I'm understanding correctly, the primary motivation for the double relayer payment is that we don't have a good way to go back and find a deposit from long ago, so we can't distinguish between a prefill and double deposit. But I also think the most likely failure mode is a double transaction submission, where the deposits are very close together, so if we could protect users in that case, I think this sharp edge wouldn't be too painful for users.

Thoughts?

UMIPs/umip-179.md

pxrl · 2025-01-30T09:36:42Z

Based on my current read of this UMIP, it's unclear how we deal with duplicate deposits.
I think we have 2 options:

If a deposit occurs within the bundle block range and one or more deposits with an identical relay hash occurs < X seconds before the deposit in question, a refund is generated for the later deposit.

If filled, only the first deposit is filled. All other deposits generate a relayer repayment, but no additional fill. If unfilled, each deposit is refunded.

I think 2 is a rephrasing of how I understand duplicate deposits should be handled:

2b. All expired, unfilled deposits are refunded

this is what I recommend to keep the logic simple. It's also good UX for duplicate depositors. It does not modify the fill logic (all fills matched with deposits are refunded). And your latter point "All other deposits generate a relayer repayment, but no additional fill. If unfilled, each deposit is refunded." is how I believe we handle "pre-fills" now.

I'm also leaning towards (2) in absence of any more comprehensive solution to it (...I haven't been able to think of one).

I'm not sure it's the best UX for depositors though - it's painful in the case they make a duplicate deposit and the repayment gets credited to the relayer of the original fill, but I think this is caveat emptor when using a function called unsafeDeposit(). The obligation lies with the depositor to avoid submitting a duplicate deposit, so we just need to provide clear guidance about the need to pre-emptively verify the FillStatus on the destination chain when selecting their depositNonce. Maybe in future we can find a better way to handle this.

I think (2) also allows us to settle on the simplest definition of what a filled relay is - when a deposit's RelayData hash has state Filled or ReplacedSlowFill on the destination chain, then it is considered Filled. This is deliberately ignorant of RelayData duplicates, collisions and even timing.

[edit]: There is one sticking point with the above proposed definition - it works poorly when the deposit is generated once, but is incidentally submitted more than once. This can be done by the depositor (standard deposit-first scenario) and can happen within the quoteTimestampBuffer, which (per present quoteTimestampBuffer configuration of 3600 seconds) guarantees that the duplicate events should be emitted within the proposer's lookback.

Scenarios to consider are:

Duplicate deposits are submitted within the same bundle block range.
Duplicate deposits are submitted in adjacent bundle block ranges.

The quoteTimestamp parameter should protect against accidental re-submitted deposits being further apart than that, though we have no prescribed rules for how quoteTimeBuffer can be set relative to the proposal challenge window.

The best I can think of at the moment is this convoluted description (edited after Nick feedback):

A Deposit is considered Filled when the destination SpokePool's FillStatus for the Deposit RelayData has state Filled.

Note:

In case multiple identical Deposits occur on a given origin SpokePool within the same or adjacent Bundle Block Ranges, only the initial Deposit shall be considered Filled. Subsequent Deposits within this range that are inherently unable to be filled on the destination chain shall be refunded.

nicholaspai · 2025-01-30T16:15:22Z

A Deposit is considered Filled when the Deposit RelayData hash has state Filled or ReplacedSlowFill at the destination SpokePool.

I think you're confusing the FillStatus type from the FillType type:

export enum FillStatus {
  Unfilled = 0,
  RequestedSlowFill,
  Filled,
}

export enum FillType {
  FastFill = 0,
  ReplacedSlowFill,
  SlowFill,
}

We can probably change this to be "A Deposit is considered Filled when the relayFillStatuses() for the relay data hash returns Filled on the destination SpokePool"

nicholaspai · 2025-01-31T01:07:35Z

Based on my current read of this UMIP, it's unclear how we deal with duplicate deposits.
I think we have 2 options:

If a deposit occurs within the bundle block range and one or more deposits with an identical relay hash occurs < X seconds before the deposit in question, a refund is generated for the later deposit.

If filled, only the first deposit is filled. All other deposits generate a relayer repayment, but no additional fill. If unfilled, each deposit is refunded.

I think 2 is a rephrasing of how I understand duplicate deposits should be handled:
2b. All expired, unfilled deposits are refunded
this is what I recommend to keep the logic simple. It's also good UX for duplicate depositors. It does not modify the fill logic (all fills matched with deposits are refunded). And your latter point "All other deposits generate a relayer repayment, but no additional fill. If unfilled, each deposit is refunded." is how I believe we handle "pre-fills" now.

I'm also leaning towards (2) in absence of any more comprehensive solution to it (...I haven't been able to think of one).

I'm not sure it's the best UX for depositors though - it's painful in the case they make a duplicate deposit and the repayment gets credited to the relayer of the original fill, but I think this is caveat emptor when using a function called unsafeDeposit(). The obligation lies with the depositor to avoid submitting a duplicate deposit, so we just need to provide clear guidance about the need to pre-emptively verify the FillStatus on the destination chain when selecting their depositNonce. Maybe in future we can find a better way to handle this.

I think (2) also allows us to settle on the simplest definition of what a filled relay is - when a deposit's RelayData hash has state Filled or ReplacedSlowFill on the destination chain, then it is considered Filled. This is deliberately ignorant of RelayData duplicates, collisions and even timing.

[edit]: There is one sticking point with the above proposed definition - it works poorly when the deposit is generated once, but is incidentally submitted more than once. This can be done by the depositor (standard deposit-first scenario) and can happen within the quoteTimestampBuffer, which (per present quoteTimestampBuffer configuration of 3600 seconds) guarantees that the duplicate events should be emitted within the proposer's lookback.

Scenarios to consider are:

Duplicate deposits are submitted within the same bundle block range.

Duplicate deposits are submitted in adjacent bundle block ranges.

The quoteTimestamp parameter should protect against accidental re-submitted deposits being further apart than that, though we have no prescribed rules for how quoteTimeBuffer can be set relative to the proposal challenge window.

The best I can think of at the moment is this convoluted description (edited after Nick feedback):

A Deposit is considered Filled when the destination SpokePool's FillStatus for the Deposit RelayData has state Filled.

Note:

In case multiple identical Deposits occur on a given origin SpokePool within the same or adjacent Bundle Block Ranges, only the initial Deposit shall be considered Filled. Subsequent Deposits within this range that are inherently unable to be filled on the destination chain shall be refunded.

The rule I've implemented so far in across-protocol/sdk#835 is:

When a fill is validated and due for a refund in a bundle, look for any duplicate deposits matching the fill (i.e. not including the original deposit) that are in the same or a prior bundle, and refund these duplicate deposits
- These duplicates can no longer get refunded as expired deposits since the relay data hash is Filled on the destination SpokePool, and these deposits cannot trigger pre-fill refunds anymore
When a deposit is sent that triggers a pre-fill refund, look for any duplicate deposits in the same bundle as this deposit and refund these duplicate deposits
- These duplicate deposits would have otherwise triggered duplicate pre-fill refunds, resulting in a transfer of funds from duplicate depositors to pre-fillers, so we might as well repay the depositor in this case
- If a deposit triggers a pre-fill refund and its the only deposit in the bundle, then the depositor's funds will pay the pre-filler

These rules do not pay duplicate depositors who are:

unlucky to send duplicate deposits across two different bundles that match with a pre-fill and are the only deposit in that bundle. If two or more duplicate deposits land in the same

I think this scenario is very unlikely if you consider how deposits are designed to work with pre-fills:

We expect pre-fill depositors to send deposits in batches, in close succession to each other, so the chance of sending these deposits in different bundles is low. The depositor can also remedy the situation by making sure they send batch duplicate deposits in at least batches of 2. This way all duplicate deposits get refeunded. Or...the depositor can just not send duplicate deposits...

mrice32 · 2025-01-31T05:23:04Z

Is all this special logic worthwhile if depositors can just get unlucky with bundle timing and lose their funds anyway? I think, in general, we should aim to make as little depend on bundle block ranges as possible, since they can be manipulated by the proposer and (even if non-malicious) are unpredictable for users/relayers.

My thinking is that the following logic would work well and require no additional lookback. It is similar to the casework above, but it combines the two cases and decouples the duplicate refund logic from the fill status.

We (already) grab deposits from 6 hours before the start of the bundle block range to the end of the block range.
For each deposit in our bundle block range, we check if there are one or more deposits that are earlier (block number or event index if the block number is equivalent), have a matching relay hash, and have a block.timestamp within 6 hours of the deposit in question.
If so, enque a refund for this deposit. Remove it from consideration for any other actions (matching fills, refund after expiry, etc).
... continue the bundling process as usual.

Thoughts? Do you think this works?

nicholaspai · 2025-01-31T12:38:24Z

UMIPs/umip-179.md

+#### Note
+- If the `Deposit` event specifies `outputToken` 0x0 (i.e. the Zero Address), the equivalent SpokePool token on the destination chain shall be substituted in. For the purpose of determining `RelayData` equivalency, the updated/substituted `outputToken` shall be used in place of 0x0.
+- `RelayData` equality can be determined by comparing the keccak256 hashes of two `RelayData` objects.
+- Fills of type `SlowFill` are valid, but are not relevant for computing relayer repayments.


For SlowFill-ed deposits where the fill is a pre-fill, we should ideally refund the depositor. Otherwise there is no way for the depositor to get funds back. We can essentially assume deposits sent after a slow fill are always duplicates, by definition for that slow fill to have been created

pxrl · 2025-01-31T21:06:07Z

So I think the duplicate deposit rules we settled on are:

if duplicate deposits are sent in the same bundle as a fill, refund the duplicate deposits

if duplicate deposits are sent after a fill such that they trigger a pre-fill refund, send a pre-fill refund for each such deposit

a single deposit can only result in a single payout: to the depositor or the filler, depending on the timing of the deposit and the fill

The last bullet point is unsatisfying but this ruileset is simple to implement, unlike one where we'd want to remove the non-determinism for the depositor

Updated version:

So we are dealing with duplicate deposits in two cases:

duplicates are sent in the same bundle or in a prior bundle to the fill
- In this case, when we see the fill:
  - If the fill is not a SlowFill, send duplicate deposits to filler
  - If the fill is a slow fill, send duplicate deposits back to depositor
duplicates are sent in a bundle after the fill’s bundle
- If the fill is not a SlowFill, send duplicate deposits to filler
- If the fill is a slow fill, send duplicate deposits back to depositor

So in all cases, duplicate deposits only get refunded once the deposit gets filled (or has already been filled at the time the deposit is mined):

duplicate deposits go to depositor if the fill is a SlowFill, Or
the filler

nicholaspai · 2025-01-31T23:39:20Z

UMIPs/umip-179.md

+1. The `Deposit` is identical with another `Deposit`.
+2. The destination chain `FillStatus` for the `Deposit` is `Filled`.
+3. The destination chain `Fill` `FillType` was `SlowFill`.
+4. The destination chain `Fill` occurred within the current `BundleBlockRange`.


Do all these conditions have to happen? If so we should probably explicitly state that above the list or adding an AND after each point

nicholaspai · 2025-01-31T23:41:06Z

UMIPs/umip-179.md

 - The AcrossConfigStore contract shall be used to identify the correct rate model, instead of a `RateModelStore` contract.
 - The `HubPool` `liquidityUtilizationCurrent()` and `liquidityUtilizationPostRelay()` functions shall be used instead of the `BridgePool` variant.
 - The event `inputToken` shall be mapped from the SpokePool address to a HubPool `l1Token` address by following the matching procedure outlined above.
- The LP fee is computed between the `originChainId` and `FilledV3Relay.repaymentChainId` where the `relayExecutionInfo.FillType != SlowFill` and `FilledV3Relay.destinationChainId` otherwise.
+- The LP fee is computed between the `originChainId` specified by the `Deposit` and `repaymentChainId` specified by the relayer, where the `relayExecutionInfo.FillType != SlowFill` and the Fill `destinationChainId` otherwise.


We actually collect LP fees on each SlowFill's execution, not sure if that was a mistake from the previous version but we should note that. SlowFill's pay out inputAmount - lpFee to the recipient and when and if the the SlowFill gets executed, the LP fee gets accumulated to the bundleLpFees entry in the PoolRebalanceRoot

nicholaspai · 2025-01-31T23:41:50Z

UMIPs/umip-179.md

- The `fillDeadline` timestamp shall be resolved to a block number on the destination chain in order to determine inclusion within the target block range.
+- The `fillDeadline` timestamp shall be resolved to a block number on the destination chain in order to determine inclusion within the `Bundle Block Range`.
+
+### Finding Unfillable Deposits


Would a better name be Duplicate Slow Filled Deposits ?

mrice32

I've done a pass. Looks mostly good, but I think we may missing the SlowFill duplicate handling.

UMIPs/umip-179.md

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

GIgako19929 · 2025-02-05T22:56:32Z

This pull request includes significant updates to the Across v3 protocol documentation in UMIPs/umip-179.md. The changes introduce new data types, update existing data types, and define new events while marking some for deprecation. Additionally, the documentation is enhanced with clearer definitions and notes for better understanding.

Data Types and Definitions Updates:

Introduced V3RelayData and V3RelayDataLegacy types, specifying bytes32 representation for addresses to interface with non-EVM chains. ([UMIPs/umip-179.mdL42-R85](https://github.com/UMAprotocol/UMIPs/pull/611/files#diff-2ec711365301885a6b023c81b242ef98007a07639b8b26e8e29fbdb846040397L42-R85))
Added V3RelayExecutionParams type for relayers or executors when completing a fill. ([UMIPs/umip-179.mdL42-R85](https://github.com/UMAprotocol/UMIPs/pull/611/files#diff-2ec711365301885a6b023c81b242ef98007a07639b8b26e8e29fbdb846040397L42-R85))
Updated FillStatus mapping to include hashed V3RelayData for querying fill status. ([UMIPs/umip-179.mdL42-R85](https://github.com/UMAprotocol/UMIPs/pull/611/files#diff-2ec711365301885a6b023c81b242ef98007a07639b8b26e8e29fbdb846040397L42-R85))

Event Updates:

Defined new events: FundsDeposited, RequestedSpeedUpDeposit, RequestedSlowFill, and FilledRelay. ([UMIPs/umip-179.mdL105-L118](https://github.com/UMAprotocol/UMIPs/pull/611/files#diff-2ec711365301885a6b023c81b242ef98007a07639b8b26e8e29fbdb846040397L105-L118))
Marked events for deprecation: V3FundsDeposited, RequestedSpeedUpV3Deposit, FilledV3Relay, and RequestedV3SlowFill. ([UMIPs/umip-179.mdL105-L118](https://github.com/UMAprotocol/UMIPs/pull/611/files#diff-2ec711365301885a6b023c81b242ef98007a07639b8b26e8e29fbdb846040397L105-L118))
Enhanced event definitions with additional fields and notes for better clarity. ([UMIPs/umip-179.mdL128-R217](https://github.com/UMAprotocol/UMIPs/pull/611/files#diff-2ec711365301885a6b023c81b242ef98007a07639b8b26e8e29fbdb846040397L128-R217))

Notes and Clarifications:

Added detailed notes to explain the purpose and usage of various fields and events. ([[1]](https://github.com/UMAprotocol/UMIPs/pull/611/files#diff-2ec711365301885a6b023c81b242ef98007a07639b8b26e8e29fbdb846040397L84-R109), [[2]](https://github.com/UMAprotocol/UMIPs/pull/611/files#diff-2ec711365301885a6b023c81b242ef98007a07639b8b26e8e29fbdb846040397L128-R217))
Updated notes to indicate the unchanged format of certain leaves from Across v2 and expanded utility in Across v3. ([[1]](https://github.com/UMAprotocol/UMIPs/pull/611/files#diff-2ec711365301885a6b023c81b242ef98007a07639b8b26e8e29fbdb846040397L177-R244), [[2]](https://github.com/UMAprotocol/UMIPs/pull/611/files#diff-2ec711365301885a6b023c81b242ef98007a07639b8b26e8e29fbdb846040397L190-R305))

Definitions Section:

Added a new section defining key terms such as Deposits, Fills, Slow Fill Requests, RelayData, Bundle Block Range, and Fill Status. ([UMIPs/umip-179.mdL190-R305](https://github.com/UMAprotocol/UMIPs/pull/611/files#diff-2ec711365301885a6b023c81b242ef98007a07639b8b26e8e29fbdb846040397L190-R305))

These changes aim to improve the clarity and functionality of the Across v3 protocol documentation, ensuring that users and developers can better understand and utilize the protocol.

GIgako19929

``

pxrl requested review from bmzig, mrice32 and nicholaspai January 17, 2025 21:13

pxrl commented Jan 17, 2025

View reviewed changes

UMIPs/umip-179.md Outdated Show resolved Hide resolved

Describe pre-fills

43b7aef

pxrl commented Jan 20, 2025

View reviewed changes

pxrl added 3 commits January 20, 2025 15:59

Cleanup

1abce9d

Add origin chain deposit constraint

debf09d

Describe relayer repaymnet corner cases

294518d

pxrl mentioned this pull request Jan 20, 2025

improve(BundleDataClient): Support pre-fills across-protocol/sdk#823

Closed

nicholaspai reviewed Jan 21, 2025

View reviewed changes

pxrl and others added 2 commits January 21, 2025 10:22

Minor wording tweak

2880a44

Co-authored-by: nicholaspai <[email protected]>

Align block range terminology

67dba76

Reinis-FRP reviewed Jan 21, 2025

View reviewed changes

UMIPs/umip-179.md Outdated Show resolved Hide resolved

Clarify repayments on lite chains

a95d437

pxrl commented Jan 21, 2025

View reviewed changes

UMIPs/umip-179.md Show resolved Hide resolved

Add invalid repayment chain clause

fde6509

pxrl changed the title ~~[WIP] feat: Support pending SpokePool upgrades~~ feat: Support pending SpokePool upgrades Jan 21, 2025

pxrl marked this pull request as ready for review January 21, 2025 15:29

nicholaspai reviewed Jan 21, 2025

View reviewed changes

UMIPs/umip-179.md Outdated Show resolved Hide resolved

bmzig reviewed Jan 21, 2025

View reviewed changes

UMIPs/umip-179.md Show resolved Hide resolved

bmzig reviewed Jan 21, 2025

View reviewed changes

UMIPs/umip-179.md Outdated Show resolved Hide resolved

pxrl added 2 commits January 21, 2025 22:25

Clarify FilledRelay messageHash

e6e4980

Clarify behaviour on invalid depositor / relayer addresses

cab40f8

pxrl commented Jan 22, 2025

View reviewed changes

nicholaspai mentioned this pull request Jan 23, 2025

feat(BundleDataClient): Support refunds for pre-fills/slow-fill-requests and duplicate deposits across-protocol/sdk#835

Merged

mrice32 reviewed Jan 23, 2025

View reviewed changes

pxrl dismissed nicholaspai’s stale review via 3a7815d January 30, 2025 08:23

pxrl added 2 commits January 30, 2025 09:24

Apply suggestions from code review

7ddf471

Update UMIPs/umip-179.md

1fe92dc

nicholaspai reviewed Jan 31, 2025

View reviewed changes

pxrl added 5 commits January 31, 2025 21:35

Updates

4c99a3e

Fix RequestedSlowFill event

7edfba7

Describe slow pre-fills

d6eb604

Tweak

3405adc

Typos

7911226

Update for duplicates & refunds

afc8eb9

nicholaspai reviewed Jan 31, 2025

View reviewed changes

mrice32 reviewed Feb 2, 2025

View reviewed changes

UMIPs/umip-179.md Outdated Show resolved Hide resolved

UMIPs/umip-179.md Outdated Show resolved Hide resolved

mrice32 added 4 commits February 2, 2025 17:09

Update UMIPs/umip-179.md

33541bc

Update UMIPs/umip-179.md

30439e9

WIP

3ddd03b

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

update language around migration

b221c6f

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

mrice32 force-pushed the pxrl/umip179-1 branch from a72b9f1 to b221c6f Compare February 3, 2025 19:13

nicholaspai approved these changes Feb 3, 2025

View reviewed changes

bmzig approved these changes Feb 3, 2025

View reviewed changes

mrice32 merged commit 55791eb into master Feb 3, 2025

mrice32 deleted the pxrl/umip179-1 branch February 3, 2025 19:26

GIgako19929 reviewed Feb 5, 2025

View reviewed changes

feat: Support pending SpokePool upgrades #611

feat: Support pending SpokePool upgrades #611

Conversation

pxrl commented Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

nicholaspai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pxrl Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pxrl Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pxrl Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrice32 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pxrl commented Jan 30, 2025 • edited Loading

nicholaspai commented Jan 30, 2025

nicholaspai commented Jan 31, 2025

mrice32 commented Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

pxrl commented Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrice32 left a comment

Choose a reason for hiding this comment

GIgako19929 commented Feb 5, 2025

Data Types and Definitions Updates:

Event Updates:

Notes and Clarifications:

Definitions Section:

GIgako19929 left a comment

Choose a reason for hiding this comment

pxrl commented Jan 17, 2025 •

edited

Loading

pxrl Jan 21, 2025 •

edited

Loading

pxrl Jan 21, 2025 •

edited

Loading

pxrl Jan 21, 2025 •

edited

Loading

pxrl commented Jan 30, 2025 •

edited

Loading

mrice32 commented Jan 31, 2025 •

edited

Loading

pxrl commented Jan 31, 2025 •

edited

Loading