-
Notifications
You must be signed in to change notification settings - Fork 20
feat: remove metadata field
#954
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: remove metadata field
#954
Conversation
a7ccdbe to
1009498
Compare
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.
Pull Request Overview
This PR removes the deprecated Metadata field from certificates in the AggKit framework as it is no longer needed for AggLayer communication from version 0.7.0 onwards. The change introduces breaking changes to certificate structure while maintaining backward compatibility by using ZeroHash in place of the removed metadata field for hash calculations.
- Removal of
Certificate.Metadatafield from the certificate structure - Updated certificate ID and signing hash calculations to use
ZeroHashinstead of metadata - Removed metadata validation logic and updated documentation to reflect the changes
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/aggsender_validator.md | Updated documentation to remove metadata validation references |
| cmd/run.go | Removed RPC services line for validator component |
| aggsender/validator/* | Removed metadata validation logic and updated test certificates |
| aggsender/flows/* | Updated certificate building to remove metadata field assignment |
| aggsender/db/aggsender_db_storage_test.go | Updated test certificates to remove metadata field |
| aggsender/aggsender_validator.go | Removed RPC service registration and related imports |
| agglayer/types/* | Removed metadata field from Certificate struct and updated JSON marshaling |
| agglayer/grpc/* | Updated gRPC conversion functions to remove metadata handling |
Comments suppressed due to low confidence (1)
cmd/run.go:1
- The line that appends RPC services from the validator has been removed, but the GetRPCServices method is still being called. This will cause a compilation error since the method has been removed from the AggsenderValidator struct.
package main
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.
Update PR description explaining that you remore RPC service for validator
@joanestebanr Updated. |
659619a to
4c46a9d
Compare
|
2da8c36
into
feat/aggsender-multisig
## 🔄 Changes Summary Enable the AggSender to work with multiple validator nodes in a committee-based validation system. The implementation includes validator services, multisig committee management, certificate validation improvements, and enhanced gRPC communication protocols. **Multisig Committee Support:** - Added `MultisigCommittee` type to manage signer sets and enforce signature thresholds. - Signers are represented as `SignerInfo` structs with both address and URL for improved context and error reporting. - Committee construction validates non-empty membership and non-zero threshold, preventing misconfiguration. - Dynamic signer management: methods for adding signers, duplicate checks by address and URL. **Aggsender Validator Refactor:** - The Aggsender certificate validation flow was refactored to integrate multisig logic. - The multisig validation logic is applicable to both `PP` and `FEP` certificates. - Certificate validation now checks for contiguous certificates, last L2 block, and settlement status using new queries. - Import bridge exit proof verification is handled via new logic using `verifyClaimProofs`, ensuring only valid proofs pass. **Certificate Metadata removal:** - It is gone from the agglayer and therefore it is not sent anymore from the aggsender either. - Only thing worth noting is that, when calculating `CertificateID`, instead of metadata field, which was used previously, we now use `ZeroHash`. **Smart contracts integration:** - **AggchainFEP contract:** Removed querying of `TrustedSequencer` address and rely on the signers committee instead - **AggchainBase contract:** Retrieve the multisig committee from the `AggchainBase` contract **Agglayer integration:** - Invoke the `GetNetworkState` API from agglayer to get the latest settled imported bridge exit info. - Multisig is populated into the certificate and sent to the Agglayer's `SendCertificate` gRPC endpoint ##⚠️ Breaking Changes - 🛠️ **Config**: Make sure that `Mode` on the `Validator` and `AggSender` are the same. - 🔌 **API/CLI**: `aggkit` version (`v0.7.0`) that supports `multisig` will now require updated contracts to run. At least version `v12.1.0-rc.3` of `agglayer-contracts`, and a new version of `agglayer` which supports `multisig`, which is the `v0.4.0` of `agglayer`. - 🗑️ **Deprecated Features**: Aggsender Phase II validator signing logic ## 📋 Config Updates - Added `AggSender.RequireCommitteeMembershipCheck = false` parameter, which defines if a check on `aggsender proposer` startup will be performed to see if the proposer is in the `multisig` committee. - Added `Validator.RequireCommitteeMembershipCheck = {{AggSender.RequireCommitteeMembershipCheck}}` parameter, which defines if a check on `aggsender validator` startup will be performed to see if the validator is in the `multisig` committee. - Added `Validator.Mode = "PessimisticProof"` parameter, which acts the same as the `AggSender.Mode`. It tells the validator that the network is a `PP` network or an `FEP` network. It has to be the same as on `aggsender proposer`. - Added `Validator.FEPConfig.SovereignRollupAddr = "{{AggSender.SovereignRollupAddr}}" parameter which is the address of the `AggchainFEP` rollup on L1 for given network for which validator is running. - Added `Validator.FEPConfig.RequireNoBlockGap = {{AggSender.RequireNoFEPBlockGap}}, which acts the same as the given paremeter on `AggSender` (proposer) config, and tells the validator if gaps in blocks in certificates are allowed in `FEP` network. ```toml [AggSender] RequireCommitteeMembershipCheck = false [Validator] # PessimisticProof or AggchainProof Mode = "PessimisticProof" RequireCommitteeMembershipCheck = {{AggSender.RequireCommitteeMembershipCheck}} [Validator.FEPConfig] SovereignRollupAddr = "{{AggSender.SovereignRollupAddr}}" RequireNoBlockGap = "{{AggSender.RequireNoFEPBlockGap}}" ``` ## ✅ Testing - 🤖 **Automatic**: `aggkit` CI - 🖱️ **Manual**: [Optional: Steps to verify] ## 🐞 Issues - Closes #792 ## 🔗 Related PRs - #814 - #832 - #838 - #839 - #843 - #842 - #846 - #858 - #847 - #865 - #861 - #863 - #875 - #876 - #881 - #877 - #898 - #920 - #913 - #926 - #945 - #951 - #954 - #957 - #955 - #974 - #978 - #985 - #989 - #984 - #998 - #1017 - #1028 - #1034 - #1024 - #1052 - #1067 - #1068 - #1050 - #1071 - #1072 - #1060 - #1087 - #1077 - #1073 --------- Co-authored-by: Goran Rojovic <[email protected]> Co-authored-by: Goran Rojovic <[email protected]> Co-authored-by: Joan Esteban <[email protected]> Co-authored-by: Rachit Sonthalia <[email protected]> Co-authored-by: Arpit Temani <[email protected]> Co-authored-by: Copilot <[email protected]>



🔄 Changes Summary
This PR removes the
Certificate.Metadatafield completely from theaggkit, as it is no longer needed to be sent to theagglayerfrom version0.7.0ofaggkit.Only thing worth noting is that, when calculating
CertificateID, instead ofmetadatafield, which was used previously, we now useZeroHash.This PR also updates the certificate
hashto sign on theproposerandvalidator. Now, when we don't haveaggchain paramsfield in certificate, we use aZeroHashinstead (as per this and this code on `agglayer).This PR removes the
validate_dbfeature from theaggsender-validatorwhich was used in local testing the test databases that their certificates are valid. Since now, we needagglayerGetNetworkStatusAPI to get the latest settled claim, this feature now does not make sense, and is removed. Along side with thevalidate_db, theaggsender-validatorrpcendpoints are removed which were used for testing, to test the localdbvalidity.NA
📋 Config Updates
NA
✅ Testing
aggkitCI🐞 Issues
metadatafield modification whenmultisigand nomultisig#939