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

feat(protocol): redesign Bridge fee & gasLimit and remove 2-step processing #16739

Merged
merged 25 commits into from Apr 17, 2024

Conversation

dantaik
Copy link
Contributor

@dantaik dantaik commented Apr 15, 2024

Objectives

  • Allow users to specify the message's fee to a large value without worrying about that all the fees will be taken by the relayer. The relayer should only make a small profit if the block's base fee is smaller than the message's specified max fee per gas, which is message.fee /message.gasLimit; the relayer will also only charge for the actual gas used.

  • Allow the relayer to make simple decision regarding whether it should process a message: if block.basefee <= message.fee /message.gasLimit, process the message, otherwise, no.

  • Allow the relayer to use a really large gas limit to transact processMessage without worrying about the actual invocation will eat all gas. The relayer can also simply just use message.gasLimit as the gas limit for his processMessage call, or with a reasonable overhead.

Modifications

  • removed two step processing to simplify design/impl, remove suspend message feature.

  • message.refundTo is deprecated, the destOwner field is used instead.

  • gasLimit no longer means the invocation gas limit, it covers the entire processMessage call. The gas limit for the message invocation is calculated as:

     invocationGasLimit = message.gasLimit - gasDeductedForCallData - _GAS_RESERVE
    • gasDeductedForCallData is the call data gas reseve for the message struct itself, we reserve 32 gas per byte as a message may be processed in two steps.

    • _GAS_RESERVE is a constant value, all processing gas cost will use this part of gas reserve, including the calldata for proof.

      The new implementation ensures even if the relayer specific a large gas limit for his processMessage transaction, the actual invocation will only use up to invocationGasLimit gas.

      @cyberhorsey To calculate the proper message.gasLimit, the relayer can use Bridge.getMessageMinGasLimit(message) + expectedInvocationGasLimit.

  • refund 20000 gas per cache operation.

Copy link

vercel bot commented Apr 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
guardian-dashboard-hekla ✅ Ready (Inspect) Visit Preview Apr 17, 2024 4:57am

Copy link

openzeppelin-code bot commented Apr 15, 2024

feat(protocol): redesign Bridge fee & gasLimit and remove 2-step processing

Generated at commit: 0ea90a98046754a361bae51c194bbfabfde438a4

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
2
2
0
4
39
47
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

@dantaik
Copy link
Contributor Author

dantaik commented Apr 16, 2024

@Brechtpd please take another look.

I'm considering the removal two-step processing. The goal of introducing this feature is to handle bridge bugs, but its very existence may introduce its own bugs....

See #16744

@dantaik dantaik marked this pull request as ready for review April 16, 2024 04:19
@dantaik dantaik changed the title feat(protocol): change some internal design in bridge feat(protocol): change some internal design in bridge [do not merge] Apr 16, 2024
@dantaik dantaik changed the title feat(protocol): change some internal design in bridge [do not merge] feat(protocol): change Bridge fee and gas limit design Apr 16, 2024
- removed "refundTo" and "memo" from messages and vaults.
- reordered field in bridge messagen and vault's op structs to make it use fewer slots
@dantaik dantaik changed the title feat(protocol): change Bridge fee and gas limit design feat(protocol): change Bridge fee and gas limit design and removed 2-step processing Apr 16, 2024
@dantaik dantaik changed the title feat(protocol): change Bridge fee and gas limit design and remove 2-step processing feat(protocol): redesign Bridge fee & gasLimit and remove 2-step processing Apr 17, 2024
Copy link
Contributor

@Brechtpd Brechtpd left a comment

Choose a reason for hiding this comment

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

Will go over the code again later today, but everything looks good so approving so I'm not blocking anything.

@dantaik dantaik removed the request for review from cyberhorsey April 17, 2024 12:03
@dantaik dantaik added this pull request to the merge queue Apr 17, 2024
Merged via the queue into main with commit 3049b0c Apr 17, 2024
7 checks passed
@dantaik dantaik deleted the bridge_improvement branch April 17, 2024 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants