-
Notifications
You must be signed in to change notification settings - Fork 19
feat: remove ger, unset claims #883
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
Conversation
## 🔄 Changes Summary Fields `CreatedAt` and `UpdatedAt` on certificates are purely `aggsender` constructs used for debugging purposes and for logging. `CreatedAt` field is a part of the certificate `Metadata` field, which will be removed in Phase III of `aggsender multi sig` feature. Because of this, only `aggsender proposer` `db` will save `CreatedAt` and `UpdatedAt` fields, but when resyncing its state with `agglayer` (when starting from scratch), it will not have this field, since `Metadata` will be removed, so we need to support these fields to be 0 in this case. The PR changed the functions that print and log these fields, so that when they are 0, we print `N/A` string. ##⚠️ Breaking Changes - NA ## 📋 Config Updates - NA ## ✅ Testing - 🤖 **Automatic**: `aggkit` CI tests are enough to test this ## 🐞 Issues - Closes #801
…a` (#838) ## 🔄 Changes Summary This PR introduces a new interface called `CertificateQuerier` which, for now, has only one function called `GetLastSettledCertificateToBlock` which calculates the last settled block from a previously settled certificate. This will be used on the validator, to calculate what was the last settled block, so we can figure out from which block a new certificate that is being verified is built. It will also be used on startup from scratch, when `aggsender` syncs its state with `agglayer`, so we can save the `ToBlock` of the last settled certificate in the `db`. The motivation behind this PR is to not use the `Metadata` field on the certificate, since it will not be signed by the proposer and validators, hence, it can be faked, but deduce the block range from other certificate data. ## How the last settled block is calculated The last settled block in the `CertificateQuerier` is calculated like this: - First it will check the `NewLocalExitRoot` field on the certificate, if it is not the first `empty` LER, meaning, there were some bridge exits in the certificate, or in certificates before it, it will get the block num on which the `NewLocalExitRoot` hash was added on `L2` by calling the `L2 bridge syncer`. - Secondly, it will call the `agglayer` endpoint to get the latest settled imported bridge exit. Note that this PR added only the endpoint sceleton, since this is not yet implemented on `agglayer`. This endpoint will return the `GlobalIndex` of the last settled imported bridge exit (claim), and we will use that to get the claim from the `L2 bridge syncer` since `GlobalIndex` is unique per claim. - Thirdly, it will ping the `AggchainFEP` rollup contract to get the last settled `L2` block from it (which is gotten from the `aggchain proof` in the `FEP` certificates). If the network is not `FEP` this call will just return 0. - At the end, it will compare the 3 blocks gotten in previous steps, and choose the max value from it. We need all three values because, PP certificates can either contain only bridge exits, or only imported bridge exits, and FEP certificates can be empty. **IMPORTANT NOTE**: This code is not used anywhere. It will be used in subsequent PRs. ##⚠️ Breaking Changes NA ## 📋 Config Updates NA ## ✅ Testing - 🤖 **Automatic**: aggkit CI
…ta` (#839) ## 🔄 Changes Summary This PR decouples the syncing with `agglayer` on `aggsender ` startup from using the certificate `Metadata` field. Since we can only be precise with calculating the last settled block on `Settled` certificates, when syncing with `agglayer` on startup, if there are any `Pending` certificates, we will wait for them to settle, before we save it to local `db`. Also, we will ignore the last `InError` certificate, and not save it to `db` since we will just rebuild it based on what was last settled. ##⚠️ Breaking Changes NA ## 📋 Config Updates NA ## ✅ Testing - 🤖 **Automatic**: `aggkit` CI ## 🐞 Issues - Closes #798
## 🔄 Changes Summary Remove the `x-agglayer-extra-certificate-signature` context metadata that is set by the aggsender validator in phase II. ##⚠️ Breaking Changes - N/A ## 📋 Config Updates - 🧾 **Diff/Config snippet**: N/A ## ✅ Testing - 🤖 **Automatic**: CI ## 🐞 Issues - Closes #795 ## 🔗 Related PRs N/A ## 📝 Notes N/A
…842) ## 🔄 Changes Summary Expose the `LastL2BlockInCert` that denotes the ending L2 block number of the certificate currently being built by the aggsender proposer. This information is needed by the validator in order for it to figure out for which blocks span the certificate is built. ##⚠️ Breaking Changes N/A ## 📋 Config Updates N/A ## ✅ Testing - 🤖 **Automatic**: CI - 🖱️ **Manual**: N/A ## 🐞 Issues - Closes #802 ## 🔗 Related PRs N/A ## 📝 Notes N/A
## 🔄 Changes Summary This PR decouples `aggsender-validator` from using the `Metadata` field in certificate to figure out block range. The validator will now use the `certificate querier` interface to get the `ToBlock` from the last settled certificate in order to figure out if the new certificate is valid or not. ##⚠️ Breaking Changes NA ## 📋 Config Updates NA ## ✅ Testing - 🤖 **Automatic**: `aggkit` CI ## 🔗 Related PRs - #838 - #839
## 🔄 Changes Summary This PR completely removes the logic around setting `Metadata` field on certificates since it will no longer be used to determin block ranges on `aggsender`. `Metadata` field will still exist on certificates, but it will no longer be used, and a `ZeroHash` will be enforced instead. ##⚠️ Breaking Changes NA ## 📋 Config Updates NA ## ✅ Testing - 🤖 **Automatic**: `aggkit` CI ## 🐞 Issues #799 ## 🔗 Related PRs #838 #839 #846
## 🔄 Changes Summary The hash for the aggsender validator is calculated based on this [comment](agglayer/protocol-research#169 (comment)). ```rust commitment = keccak256_combine([ certificate.new_local_exit_root, commit_imported_bridge_exits: certificate.claim_hash(), // defined below certificate.height, certificate.aggchain_params, // nil value if no aggchain proof certificate_id // re-computed only on the agglayer, but free input for the PP ]); /// keccak(ib[0].global_index # ib[0].bridge_exit_hash # .. # ib[n].global_index # ib[n].bridge_exit_hash) fn claim_hash() -> Digest { keccak256_combine(self.imported_bridge_exits.iter().map(|ibe| { [ ibe.global_index.as_le_slice(), ibe.bridge_exit_hash.as_slice(), ] .concat() })) } ``` ##⚠️ Breaking Changes - 🛠️ **Config**: N/A - 🔌 **API/CLI**: N/A - 🗑️ **Deprecated Features**: N/A ## 📋 Config Updates - 🧾 **Diff/Config snippet**: N/A ## ✅ Testing - 🤖 **Automatic**: [Optional: Enumerate E2E tests] - 🖱️ **Manual**: N/A ## 🐞 Issues - Closes #804 ## 🔗 Related PRs - N/A ## 📝 Notes - N/A --------- Co-authored-by: Goran Rojovic <100121253+goran-ethernal@users.noreply.github.com>
## 🔄 Changes Summary - Bump protocol contracts and retrieve the signers URLs from the `AggchainBase` smart contract. ##⚠️ Breaking Changes - 🛠️ **Config**: N/A - 🔌 **API/CLI**: N/A - 🗑️ **Deprecated Features**: N/A ## 📋 Config Updates - 🧾 **Diff/Config snippet**: N/A ## ✅ Testing - 🤖 **Automatic**: CI ## 🐞 Issues - Closes #870 ## 🔗 Related PRs N/A ## 📝 Notes N/A
…or` (#863) ## 🔄 Changes Summary This PR adds a sanity check on the `aggsender-validator` for the `LastL2BlockInCert` field which represents the last L2 block in the proposed certificate. - `LastL2BlockInCert` can not be lower or equal to the last settled block in network. - `LastL2BlockInCert` can not be lower than the deduced `ToBlock` in the proposed certificate gotten by checking the `NewLocalExitRoot` and imported bridge exits in the certificate. ##⚠️ Breaking Changes N/A ## 📋 Config Updates N/A ## ✅ Testing - 🤖 **Automatic**: `aggkit` CI ## 🐞 Issues - Closes #805
## 🔄 Changes Summary This PR adds support for FEP certificates validation in `aggsender-validator`. ##⚠️ Breaking Changes NA ## 📋 Config Updates Added new config parameters to `Validator` config: - `Mode` - mode of the validator. Similar to regular `aggsender` (proposer) it can be `PessimisticProof` or `AggchainProof` based on if the network is a `PP` or `FEP`. - `FEPConfig` - `FEP` (`AggchainProof` mode) network specific configuration: - `SovereignRollupAddr` - address of the `AggchainFEP` contract. - `RequireNoFEPBlockGap` - true if the AggSender should not accept a gap between last block from last certificate and first block of `FEP` network. - 🧾 **Diff/Config snippet**: ```toml [Validator] # PessimisticProof or AggchainProof Mode = "PessimisticProof" [Validator.FEPConfig] SovereignRollupAddr = "{{AggSender.SovereignRollupAddr}}" RequireNoFEPBlockGap = "{{AggSender.RequireNoFEPBlockGap}}" ``` ## ✅ Testing - 🤖 **Automatic**: `aggkit` CI ## 🐞 Issues - Closes #803
## 🔄 Changes Summary Because of `Remove Injected GER` feature, the `L2 Bridge Syncer` can have (in rare cases) two `claims` with the same `global index`, where one of them gets unclaimed. Because of this, and since syncer's `db` has no constraints on `global index` field, the `aggsender-validator` needs to support this as well, when querying the `claim` based on `global index`. The `agglayer` will return a `bridge exit hash` alongside `global index` for the last settled `imported bridge exit` (`claim`), which we can use to filter out the desired `claim`. The `aggsender-validator` will query all the `claims` from the syncer based on the last settled `global index` and then filter them out by their `bridge exit hash` to find a correct one. ##⚠️ Breaking Changes NA ## 📋 Config Updates NA ## ✅ Testing - 🤖 **Automatic**: `aggkit` CI ## 🐞 Issues - Closes #878
## 🔄 Changes Summary Intregrate multisig committee in the aggsender proposer. On each certificate submission, the aggsender proposer queries the smart contract for the actual committee and delegates the certificate validation to the validators and aggregates the signatures before sending them alongside with the certificate to the `agglayer`. ## ✅ Testing - 🤖 **Automatic**: CI - 🖱️ **Manual**: N/A ## 🐞 Issues - Closes #793 ## 📝 Notes - N/A ## TODOs: - [x] Remove empty url [validation](https://github.com/agglayer/aggkit/blob/0c31ef8ff11fe183682ddd79b5fdc92ac6a00963/aggsender/config/config.go#L129-L131) as they are retrieved from the contract
df12a9c to
7de82ac
Compare
7de82ac to
d2c155e
Compare
5442380 to
8252037
Compare
Stefan-Ethernal
left a comment
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.
Generally LGTM.
joanestebanr
left a comment
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.
Coverage is below 80%, please increase it
|



🔄 Changes Summary
we need to send inserted gers and removed gers, claims and unclaims in separate fields (inserted and claims will be unfiltered and will go as it is). using these values provided by us to them, they will generate the proof and return it to us
In FEP and PP:
we need to filter unclaims from claims which are present in the same cert, if they are in different cert, we dont need to send anything. removed gers dont need to be filtered from inserted gers
More desc can be found - https://github.com/agglayer/protocol-research/issues/180
✅ Testing
Config Changes:
polygonBridgeAddr. Instead of this use [L1Config.BridgeAddr] and [L2Config.BridgeAddr] to set the respective networks bridge addresses🐞 Issues
Dependent PRs