feat(SpokePool): Remove maxCount related logic #384
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Breaking changes from existing `SpokePool` types of same name:
## RelayData
- `relayerFeePct`/ `realizedLpFeePct` replaced with `outputAmount`. The total fee is implied to be difference between `inputAmount` - `outputAmount` * `exchangeRateInputOutputTokens`
- Fill `expiryTimestamp`
- Exclusive `relayer`
## RelayExecution
- No `maxCount`
- `bool isSlowRelay` has been replaced with an enum `executionType` which is also added to the `SlowFill` struct so that creator of SlowFill leaf can specify whether this is a deposit refund or a normal slow fill. This is designed to be used by the executor to filter out slow fills with messages that it *shouldn't* execute versus deposit refunds that it *should* execute.
- Updatable deposit fields are optimistically set to:
- outputAmount
- recipient
- message
## SlowFill
- Allow slow fill creator to specify `executionType` to distinguish between deposit refunds and normal slow fills. Deposit refunds should have a `message` field set to help the `depositCallbackAddress` determine the deposit that was refunded but this should still be executed by executor. Typically executors won't execute slow fills with messages.
## FundsDeposited
- Adds `depositRefundCallbackAddress` to rest of `RelayData` params. Can be used by dataworker to create a deposit refund as a slow fill leaf.
# Optimizing compilation of contracts
Unfortunately, adding more functions to the `SpokePool` will push it over the contract bytecode size unless we decrease the optimizer. Not sure how to get around this since we want to maintain old functionality so we can only add to the size. I've bumped it down by 100x to the point where the compiler doesn't warn anymore
Co-authored-by: Paul <108695806+pxrl@users.noreply.github.com>
Signed-off-by: nicholaspai <npai.nyc@gmail.com>
Co-authored-by: Paul <108695806+pxrl@users.noreply.github.com>
Co-authored-by: Paul <108695806+pxrl@users.noreply.github.com>
Signed-off-by: nicholaspai <npai.nyc@gmail.com>
…eposited event and add new depositUSS function
Fixes #ACX-1684 and #ACX-1685
## Modifications to existing deposit function: deposit():
- At upgrade: backwards compatible deposit() function will emit the new event. The deposit2() function (with new params) will emit the new event as well. There will be no deposit function post upgrade that emits the old event. The idea behind modifying the existing function is that post-upgrade, no more “old deposits” are emitted.
- Instead of the depositor using the relayerFeePct parameter to set the relayer’s portion of the fee, depositor uses the relayerFeePct to mark a “total fee” percentage. The recipient therefore knows that they will receive `amount * (1 - relayerFeePct)` tokens. The funds deposited event emits outputTokenAmount equal to this amount * (1 - relayerFeePct).
- outputToken: new parameter, defaults to some default token address like 0x0 to indicate that the same input and output token should be used
- Should hardcode committedFiller to 0x0
- Set expiryTimestamp to MAX_UINT. We never want to process deposit refunds for “old deposits”. We should add a rule in the dataworker that deposit refunds only count for type 2 deposits.
## Provides new public function deposit2() for depositors:
- expiryTimestamp: integer
- This is going to be relative to the destination chain. Don’t allow this to be more than ~3 bundles into the future, so 12 hours maybe. This way the dataworker/relayer doesn’t have to maintain more a lookback longer than this.
- Emits FundsDeposited2 event
Signed-off-by: nicholaspai <npai.nyc@gmail.com>
Signed-off-by: nicholaspai <npai.nyc@gmail.com>
Resolves #ACX-1686
Signed-off-by: nicholaspai <npai.nyc@gmail.com>
lpFee for version 2's will use `block.timestamp` of the deposit v2 event
its ok if function interfaces take structs but ideally events do not
…s to reduce gas Emit extra event to help anyone tracking relay balances sent (i.e. updatedX params that are not used to construct relayHash)
Co-authored-by: Paul <108695806+pxrl@users.noreply.github.com>
Co-authored-by: Paul <108695806+pxrl@users.noreply.github.com>
pxrl
reviewed
Dec 19, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Supporting maxcount adds extra gas cost to soon-to-be deprecated deposit and fill functions. This PR removes them and also removes related functions like
requestRepaymentChainId.A maxCount is likely to be added in future to new deposit and fill functions (i.e. USS-functions) but we'll add them in a follow-up PR