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

chore(spec): update bridge spec #1705

Merged
merged 5 commits into from
Nov 5, 2024
Merged

Conversation

itamarreif
Copy link
Contributor

Summary

Updated the bridge spec to add more details on some of the data structures and link to the up-to-date actions.

Background

With the preparation for mainnet, some of the action names and locations have changed. The spec needed updating to reflect that.

Related Issues

Link any issues that are related, prefer full github links.

closes

@itamarreif itamarreif self-assigned this Oct 21, 2024
@itamarreif itamarreif requested a review from a team as a code owner October 21, 2024 22:59
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Oct 21, 2024
@itamarreif itamarreif force-pushed the itamarreif/bridge/docs-update branch 2 times, most recently from 0856cb1 to bd609d9 Compare October 21, 2024 23:29

The Astria sequencer implements a native bridging protocol which allows for
bridging assets from the sequencer to rollups which decide to use this protocol.

At a high level, the sequencer-to-rollup protocol works as follows:
Copy link
Collaborator

Choose a reason for hiding this comment

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

i like having the high level summary first, before delving into the code details. can you re-add?

specs/bridge.md Outdated
1. A bridge account is initialized on the sequencer with an associated rollup
ID, which is the rollup ID that the rollup reads its block data from.

2. The rollup registers the sequencer bridge account for use in its state
Copy link
Collaborator

Choose a reason for hiding this comment

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

i would say "modifies its consensus" or something like that here

specs/bridge.md Outdated
Comment on lines 47 to 48
transfer funds out of the bridge account to other sequencer accounts, such as
[`BridgeUnlock` actions.](https://github.com/astriaorg/astria/blob/d03059977c3a40590d66591c520bfda3a9b9de1c/proto/protocolapis/astria/protocol/transaction/v1/action.proto#L186)
Copy link
Collaborator

@noot noot Oct 29, 2024

Choose a reason for hiding this comment

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

Suggested change
transfer funds out of the bridge account to other sequencer accounts, such as
[`BridgeUnlock` actions.](https://github.com/astriaorg/astria/blob/d03059977c3a40590d66591c520bfda3a9b9de1c/proto/protocolapis/astria/protocol/transaction/v1/action.proto#L186)
transfer funds out of the bridge account to other sequencer accounts, eg. via
[`BridgeUnlock` actions.](https://github.com/astriaorg/astria/blob/d03059977c3a40590d66591c520bfda3a9b9de1c/proto/protocolapis/astria/protocol/transaction/v1/action.proto#L186)

also mention funds can only be transferred out via BridgeUnlock or IBC withdrawal?

specs/bridge.md Outdated
Comment on lines 104 to 105
the `Withdrawal` event. These actions are batched and signed into sequencer
transaction by rollup block, and the transaction is submitted to the sequencer.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
the `Withdrawal` event. These actions are batched and signed into sequencer
transaction by rollup block, and the transaction is submitted to the sequencer.
the `Withdrawal` event. These actions are batched by rollup block into one sequencer transaction, and the transaction is submitted to the sequencer.

specs/bridge.md Outdated
`Withdrawal` event that includes the asset, the amount, and a memo.
- the memo contains the needed information for creating an Ics20 withdrawal on
The User initiates a withdrawal from the rollup to an IBC destination using the
`Ics20Withdrawal` function of an `IAstriaWithdrawer`-compatible smart contract. This
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
`Ics20Withdrawal` function of an `IAstriaWithdrawer`-compatible smart contract. This
`withdrawToIbcChain` function of an `IAstriaWithdrawer`-compatible smart contract. This

@itamarreif itamarreif requested a review from noot October 30, 2024 16:46
specs/bridge.md Outdated
Comment on lines 29 to 30
1. The bridge withdrawer is registered with the sequencer-side bridge account,
allowing it to make withdrawals from the bridge account.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is a bit confusing, maybe rephrase to "the bridge account has an associated withdrawer address which is authorized to withdraw from the bridge account"

@itamarreif itamarreif added this pull request to the merge queue Nov 5, 2024
Merged via the queue into main with commit 3ee2d99 Nov 5, 2024
46 checks passed
@itamarreif itamarreif deleted the itamarreif/bridge/docs-update branch November 5, 2024 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bridging documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants