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(BundleDataClient): Support pre-fills #823

Closed
wants to merge 2 commits into from

Conversation

nicholaspai
Copy link
Member

@nicholaspai nicholaspai commented Jan 9, 2025

Motivation

Support use case that will become frequent once deterministic relay hashes go live where a fill precedes a deposit in real world time.

Update #1:

I'm not even sure there is a problem with refunding pre fills that occur near the end of a bundle (that precede a deposit at the beginning of the following bundle) because bundle end blocks are currently set to lag HEAD on all chains. Additionally, the bundle data client takes into account all deposits, even those right near HEAD, when validating fills. So unless the fill precedes the deposit by more than the bundle end block buffer then there should be no issue.

Problem

This opens up the possibility that even with a SpokePoolClient.query lag applied on all chains, there is a very unlucky situation where a fill is sent at the end of one bundle block range and a deposit is one of the first events in the next bundle block range. Currently, the dataworker would never issue a refund for the fill.

There are two ways around this I think and one is very complex to implement while the other is simple but increases relayer repayment latency and puts responsibility of how early to-prefill on the relayer:

  1. BundleDataClient will need to "remember" which refunds it has already issued. It can do this easily using the arweave client though this increases dependencies on Arweave. Additionally, it will be quite complex to implement the most efficient data structure. I don't love this approach because of the tradeoffs of the following approach.
  2. Increase the buffer that a fill must follow its destination chain HEAD block before the dataworker attempts to repay it. This additional buffer converted to seconds becomes the maximum amount of time that a fill can precede a deposit. I think this can be to something reasonable like 10 minutes assuming that pre-fills are only used by fillers who can guarantee somehow that a deposit lands on-chain when they want it to. Therefore the filler should be able to send a deposit after a fill in under 10 minutes in all cases.

Support use case that will become frequent once deterministic relay hashes go live where a fill precedes a deposit in real world time.

This opens up the possibility that even with a [SpokePoolClient.query lag](https://github.com/across-protocol/sdk/blob/40888c405db19f1860f0d3f22e26cc5dc541306f/src/clients/BundleDataClient/utils/PoolRebalanceUtils.ts#L28) applied on all chains, there is a very unlucky situation where a fill is sent at the end of one bundle block range and a deposit is one of the first events in the next bundle block range. Currently, the dataworker would never issue a refund for the fill.

There are two ways around this I think and one is very complex to implement while the other is simple but increases relayer repayment latency and puts responsibility of how early to-prefill on the relayer:

1. BundleDataClient will need to "remember" which refunds it has already issued. It can do this easily using the arweave client though this increases dependencies on Arweave. Additionally, it will be quite complex to implement the most efficient data structure. I don't love this approach because of the tradeoffs of the following approach.
2. Increase the buffer that a fill must follow its destination chain HEAD block before the dataworker attempts to repay it. This additional buffer converted to seconds becomes the maximum amount of time that a fill can precede a deposit. I think this can be to something reasonable like 10 minutes assuming that pre-fills are only used by fillers who can guarantee somehow that a deposit lands on-chain when they want it to. Therefore the filler should be able to send a deposit after a fill in under 10 minutes in all cases.
@nicholaspai nicholaspai requested review from mrice32, bmzig and pxrl January 9, 2025 23:03
// a filler to send a fill before a deposit in cases where the depositor is delegating transaction submission
// to the filler and giving them a signed transaction object.
const getFillBlockWithLag = (fill: V3FillWithBlock): number => {
const fillBuffer = /* FILL_BUFFER[fill.destinationChainId] ?? */ 0;
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 imagine to start we'd define a hardcoded value for this fill buffer. Technically it would need to be in the UMIP so it might even have to be a config store variable...

@pxrl
Copy link
Contributor

pxrl commented Jan 20, 2025

@nicholaspai I've been working on the UMIP and I'm wondering if we can get away with an even smaller change for this. So far I think it's feasible to just resolve these old fills via binary search, assuming that we won't be flooded with them. For this we'd also need to ensure that the proposer only ever considers deposits that also occur within its current proposal block range (i.e. so it should not consider deposits that come after).

I'm hopeful that it'll work OK but it needs some pressure testing.

UMAprotocol/UMIPs#611 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants