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.

Sign up for GitHub

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

Jump to bottom

[WIP] [fix]: Address UMIP-179 feedback #602

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

[WIP] [fix]: Address UMIP-179 feedback #602

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
867733d
[fix]: Address UMIP-179 feedback
pxrl Apr 22, 2024
a45310c
Fix repayment chain wording
pxrl Apr 22, 2024
d06ad9e
Fix 1 -> 1e18
pxrl Apr 22, 2024
4812cc3
Clarify LP fee precision
pxrl Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions UMIPs/umip-179.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,25 +244,30 @@ Each valid `FilledV3Relay` event is subject to an LP fee. The procedure for comp
- 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.

Note:
- The LP fee is typically referenced as a multiplier of the `V3FundsDeposited` `inputAmount`, named `realizedLpFeePct` elsewhere in this document.
- The LP fee as a multiplier of `inputAmount` (typically named `realizedLpFeePct` elsewhere in this document) shall be:
- Promoted to 18 decimals, where 1e18 represents 100%, and
- Truncated (zeroed) between 0 and 1e10, providing 8 decimals of effective precision.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would appreciate feedback on the wording here.

Copy link
Member

Choose a reason for hiding this comment

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

Do you think it's easier to understand if you show the math?

The LP fee is truncated via the following formula: output = input / 1e10 * 1e10

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe:

 - Available precision to 8 decimals. Truncated via [output = input / 1e10 * 1e10]


### Computing Bundle LP Fees
The bundle LP fee for a target block range on a SpokePool and token pair shall be determined by summing the applicable LP fees for each of the following validated events:
- `FilledV3Relay`

### Computing Relayer Repayments
For a validated `FilledV3Relay` event, the relayer repayment amount shall be computed as follows:
- `(inputAmount * (1 - realizedLpFeePct)) / 1e18`, where `realizedLpFeePct` is computed over the set of HubPool `l1Token`, `originChainId` and `repaymentChainId` at the HubPool block number corresponding to the relevant `V3FundsDeposited` `quoteTimestamp`.
- `(inputAmount * (1e18 - realizedLpFeePct)) / 1e18`, where `realizedLpFeePct` is computed over the set of HubPool `l1Token`, `originChainId` and `repaymentChainId` at the HubPool block number corresponding to the relevant `V3FundsDeposited` `quoteTimestamp`.
Copy link
Member

Choose a reason for hiding this comment

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

+1

- The applicable rate model shall be sourced from the AcrossConfigStore contract for the relevant `l1Token`.

If a validated `FilledV3Relay` event specifies an invalid `repaymentChainId`, the proposer shall issue repayment on the fill destination chain.
Reasons for the `repaymentChainId` to be considered invalid are:
- `repaymentChainId` is not supported by Across.
- `inputToken` is not supported as a repayment token on `repaymentChainId`.
Comment on lines +260 to +263
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think forcing repayment on the destination chain is the only safe approach here, since if the relayer fills via a contract on zkSync and the proposer proposes repayment on some other chain then the relayer will never receive their repayment.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, good point. I think that's right.


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

### Computing Slow Fill updated output amounts
For the purpose of computing the amount to issue to a recipient for a SlowFill, the relayer fee shall be nulled by applying the following procedure:
- `updatedOutputAmount = (inputAmount * (1 - realizedLpFeePct)) / 1e18`, where `realizedLpFeePct` is computed at the deposit `quoteTimestamp` between `originChainId` and `destinationChainId`.
- `updatedOutputAmount = (inputAmount * (1e18 - realizedLpFeePct)) / 1e18`, where `realizedLpFeePct` is computed at the deposit `quoteTimestamp` between `originChainId` and `destinationChainId`.

Constraint:
- The `V3FundsDeposited` `outputAmount` shall _not_ be considered when determining SlowFill amounts.
Expand Down Expand Up @@ -368,7 +373,7 @@ Each Relayer Refund Leaf shall be constructed as follows:
1. `refundAmount` descending order, then
2. `relayerAddress` ascending order (in case of duplicate `refundAmount` values).

In the event that the number of refunds contained within a Relayer Refund leaf should exceed [`MAX_RELAYER_REPAYMENT_LEAF_SIZE`]((https://github.com/UMAprotocol/UMIPs/blob/7b1a046098d3e2583abd0372c5e9c6003b46ad92/UMIPs/umip-157.md#global-constants) refunds:
In the event that the number of refunds contained within a Relayer Refund leaf should exceed [`MAX_RELAYER_REPAYMENT_LEAF_SIZE`](https://github.com/UMAprotocol/UMIPs/blob/7b1a046098d3e2583abd0372c5e9c6003b46ad92/UMIPs/umip-157.md#global-constants) refunds:
1. Additional `RelayerRefundLeaf` instances shall be produced to accomodate the excess.
2. The ordering of `refundAddresses` and `refundAmounts` shall be maintained across the ordered array of leaves.
3. Only the first leaf for a given `l2TokenAddress` shall contain a non-zero `amountToReturn`.
Expand All @@ -380,6 +385,12 @@ The set of relayer refund leaves shall be ordered according to:

The Relayer Refund Leaf `leafId` field shall be numbered according to the ordering established above, starting at 0.

If a Relayer Refund Leaf would be unable to be executed due to an ERC20 reversion when transferring the repayment token to the recipient on the destination chain, the proposer may exclude the relayer repayment from the Relayer Refund Root.
Copy link
Member

Choose a reason for hiding this comment

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

Two things:

  • Long term, I think this would be better handled in the contracts (having a data structure to store unexecuted payments that can be claimed anytime).
  • I think the way this is written, it's ambiguous. I think we should say to be excluded from the bundle, it needs to be unexecutable at the bundle end block of the chain where the repayment will take place.

Note:
- This is intended to deal with unlikely situations, such as a centralized token issuer censoring transactions concerning a relayer address, and is required to prevent deadlocking of all other relayer repayments in the same Relayer Refund Leaf.
- In the event of relayer repayment exclusion, the proposer should provide reproducible evidence that the relayer repayment would fail as part of any subsequent dispute.


Note:
- Once these leaves are constructed, they can be used to form a merkle root as described in the previous section.

Expand Down Expand Up @@ -413,7 +424,4 @@ The V3 rules defined in this UMIP will apply beginning when the VERSION field in
The Across v3 implementation is available in the Across [contracts-v2](https://github.com/across-protocol/contracts-v2) repository.

# Security considerations
Across v3 has been audited by OpenZeppelin.

Note:
- If a particular relayer refund is known to be unexecutable, it can be removed from the bundle by the proposer if a sufficient public justification is made before the proposal. This is intended to deal with unlikely situations, such as ag centralized token issuer blacklisting an address that is due a refund. If this leaf were to remain unaltered, this blacklisted address could block other addresses from recieving refunds.
Across v3 has been [audited by OpenZeppelin](https://blog.openzeppelin.com/across-v3-incremental-audit).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Across v3 has been [audited by OpenZeppelin](https://blog.openzeppelin.com/across-v3-incremental-audit).
Across v3 has been audited by OpenZeppelin, shared [here](https://blog.openzeppelin.com/across-v3-incremental-audit) and [here](https://blog.openzeppelin.com/across-v3-and-oval-incremental-audit).