-
Notifications
You must be signed in to change notification settings - Fork 343
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
Update AnyTrust docs with links and refinements #1715
base: master
Are you sure you want to change the base?
Update AnyTrust docs with links and refinements #1715
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! 🙏
I've added a few comments here and there. I'd recommend avoiding changes to concept names that are used throughout the document, since those might also be referenced in other pages of the docs, so changing only this instance might create unnecessary complexity in the language.
|
||
- the number of Committee members, and | ||
- for each Committee member, a BLS public key, and | ||
- the number of Committee signatures required. | ||
|
||
Keysets are identified by their hashes. | ||
|
||
An L1 KeysetManager contract maintains a list of currently valid Keysets. The L2 chain's Owner can add or remove Keysets from this list. When a Keyset becomes valid, the KeysetManager contract emits an L1 Ethereum event containing the Keyset's hash and full contents. This allows the contents to be recovered later by anyone, given only the Keyset hash. | ||
The SequencerInbox contract maintains `dasKeySetInfo` mapping with embedded [access controls](https://github.com/OffchainLabs/nitro-contracts/blob/fbbcef09c95f69decabaced3da683f987902f3e2/src/bridge/SequencerInbox.sol#L750-L778) for adding new keysets and invalidating existing ones. Upon addition, a L1 system contract [event](https://github.com/OffchainLabs/nitro-contracts/blob/fbbcef09c95f69decabaced3da683f987902f3e2/src/bridge/ISequencerInbox.sol#L197-L201) containing the Keyset's hash and full contents is emitted and processed by the L2 chain during [validation](https://github.com/Layr-Labs/nitro/blob/eigenda-v3.2.1/arbstate/daprovider/util.go#L194). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few things about this paragraph:
- I believe it's clearer to mention that the chain owner is the one able to add or remove keysets from the mapping (instead of mentioning
access controls
) - Since you're referencing the SequencerInbox at the beginning, you can do so too when mentioning the event that's emitted (right now it says
a L1 system contract
). - Those events are not only exactly processed by validators. They are emitted and later used to verify new signed certificates. In any case, the original text was clearer in that regard.
- The new text includes links to repositories that are not maintained by Arbitrum or OCL (Layr-Labs/nitro)
|
||
Although the API does not limit the number of Keysets that can be valid at the same time, normally only one Keyset will be valid. | ||
Although the API does not limit the number of active Keysets that can be valid at the same time, normally only one Keyset will be valid. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Active keysets that can be valid
seems a bit redundant. I'd remove active
from here.
|
||
## Data Availability Certificates | ||
|
||
A central concept in AnyTrust is the Data Availability Certificate (hereafter, a "DACert"). A DACert contains: | ||
A central concept in AnyTrust is the Data Availability Certificate (hereafter, a "DACert"). A [DACert](https://github.com/Layr-Labs/nitro/blob/eigenda-v3.2.1/arbstate/daprovider/util.go#L239-L246) contains: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference to a repo that's not maintained by Arbitrum or OCL
|
||
Because of the 2-of-N trust assumption, a DACert constitutes proof that the block's data (i.e., the preimage of the hash in the DACert) will be available from at least one honest Committee member, at least until the expiration time. | ||
Because of the `1-of-N` trust assumption, a DACert constitutes proof that the batch data (i.e., the preimage of the hash in the DACert) will be available from at least one honest Committee member, at least until the expiration time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of the `1-of-N` trust assumption, a DACert constitutes proof that the batch data (i.e., the preimage of the hash in the DACert) will be available from at least one honest Committee member, at least until the expiration time. | |
Because of the `2-of-N` trust assumption, a DACert constitutes proof that the block's data (i.e., the preimage of the hash in the DACert) will be available from at least one honest Committee member, at least until the expiration time. |
We are using the same language throughout the rest of the page, so I wouldn't change this instance alone. If we want to change "data block" to "batch" we would have to do so in all instances.
|
||
In ordinary (non-AnyTrust) Nitro, the Arbitrum sequencer posts data blocks on the L1 chain as calldata. The hashes of the data blocks are committed by the L1 Inbox contract, allowing the data to be reliably read by L2 code. | ||
The hashes of the data blocks are committed by the L1 chain's Sequencer Inbox contract, allowing the data to be reliably read by L2 chain nodes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't remove the first part of the paragraph. It makes sense to reference the way non-AnyTrust nitro works, to be compared with AnyTrust.
|
||
AnyTrust gives the sequencer two ways to post a data block on L1: it can post the full data as above, or it can post a DACert proving availability of the data. The L1 inbox contract will reject any DACert that uses an invalid Keyset; the other aspects of DACert validity are checked by L2 code. | ||
AnyTrust gives the sequencer an additional way to post batches to the L1 chain. The L1 Sequencer Inbox contract will reject any DACert that uses an invalid Keyset; other certificate validity checks are enforced during derivation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to specify what's the additional way to post a batch.
|
||
The L2 code that reads data from the inbox reads a full-data block as in ordinary Nitro. If it sees a DACert instead, it checks the validity of the DACert, with reference to the Keyset specified by the DACert (which is known to be valid because the L1 Inbox verified that). The L2 code verifies that | ||
The L2 code that reads data from the inbox reads a full-data block as in ordinary Nitro. If it sees a DACert instead, it checks the validity of the DACert, with reference to the Keyset specified by the DACert (which is known to be valid since the Sequencer Inbox verified that). The L2 chain code verifies that: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SequencerInbox is usually written without spaces.
We should be consistent with the language. L2 code
is a valid expression, but if we want to change it to another thing, we would have to change all instances.
In any case, my recommendation for an external contribution is not to change language that is correct, even if there might be a better way to express that concept, since we take a global approach within the docs, and try to use the same language across all pages. The best way to deal with changing language is to open an issue so we can discuss internally how to approach the change 🙏
|
||
- the number of signers is at least the number required by the Keyset, and | ||
- the aggregated signature is valid for the claimed signers, and | ||
- the expiration time is at least two weeks after the current L2 timestamp. | ||
|
||
If the DACert is invalid, the L2 code discards the DACert and moves on to the next data block. If the DACert is valid, the L2 code reads the data block, which is guaranteed to be available because the DACert is valid. | ||
If the DACert is invalid, the L2 code discards the DACert and moves on to the next data block. If the DACert is valid, the L2 code reads the data block, which is guaranteed to be available because the DACert is valid. There are no significant changes to fraud proving since these validations are compiled into the wasm replay script - removing the need for additional opcodes or changes to one step proving for a `READSEQUENCERINBOX` opcode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The last sentence of the paragraph seems a bit irrelevant in this context. The whole AnyTrust page refers only to the way the information is stored, which makes sense since it's the only thing that changes.
If we want to add a note about the potential impact on fraud proving, it might be better to create a new section for it, but I don't think it's necessary to include it.
@@ -68,4 +68,4 @@ When the Arbitrum sequencer produces a data batch that it wants to post using th | |||
|
|||
Once the Sequencer has collected enough signatures, it can aggregate the signatures and create a valid DACert for the (hash, expiration time) pair. The Sequencer then posts that DACert to the L1 inbox contract, making it available to the AnyTrust chain software at L2. | |||
|
|||
If the Sequencer fails to collect enough signatures within a few minutes, it will abandon the attempt to use the Committee, and will "fall back to rollup" by posting the full data directly to the L1 chain, as it would do in a non-AnyTrust chain. The L2 software can understand both data posting formats (via DACert or via full data) and will handle each one correctly. | |||
If the Sequencer fails to collect enough signatures within a few minutes, it will abandon the attempt to use the Committee, and will "fall back to rollup" by posting the full data directly to the L1 chain, as it would do in a non-AnyTrust chain. The Nitro software can understand both data posting formats (via DACert or via native Ethereum DA) and will handle each one correctly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the first mention to "native Ethereum DA" and might be confusing for the reader. A better alternative would be via complete data
).
No description provided.