Skip to content

Conversation

@goran-ethernal
Copy link
Contributor

@goran-ethernal goran-ethernal commented Sep 5, 2025

🔄 Changes Summary

This PR moves the polling of validator committee to validate a new certificate to a new file: validator_poller.go, and a new interface called ValidatorPoller.

Polling of validators is now separated into smaller functions, that contain:

  • validating the request to validate the certificate.
  • getting the validator committee.
  • check if the first member in the commitee is the proposer.
  • polling each committee member to validate and sign the certificate.
  • if one of the members is the proposer, it will sign the certificate locally.
  • gather results, and return them to the proposer.

⚠️ Breaking Changes

NA

📋 Config Updates

NA

✅ Testing

  • 🤖 Automatic: aggkit CI

🐞 Issues

Copy link
Contributor

Copilot AI left a 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 refactors the validator committee polling functionality by extracting it from the main AggSender struct into a dedicated ValidatorPoller interface and implementation. The refactoring improves separation of concerns and makes the validation logic more modular and testable.

Key Changes

  • Extracts validator committee polling logic into a new ValidatorPoller interface and validatorPoller implementation
  • Moves validation functionality from aggsender.go to dedicated validator_poller.go file
  • Adds support for self-signing when the proposer is also a committee member
  • Creates comprehensive test coverage for the new validator polling functionality

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
aggsender/validator/validator_poller.go New file implementing the ValidatorPoller interface with extracted validation logic
aggsender/validator/validator_poller_test.go Comprehensive test suite for the new validator poller functionality
aggsender/types/interfaces.go Adds ValidatorPoller interface and updates AggsenderFlow interface with Signer() method
aggsender/types/multisig_committee.go Adds ValidationRequest struct for encapsulating validation parameters
aggsender/flows/flow_pp.go Implements Signer() method for PP flow
aggsender/flows/flow_aggchain_prover.go Implements Signer() method for aggchain prover flow
aggsender/aggsender.go Refactored to use ValidatorPoller interface instead of direct validation logic
aggsender/aggsender_test.go Removes tests that are now covered by validator poller tests
aggsender/mocks/mock_validator_poller.go Generated mock for the new ValidatorPoller interface
aggsender/mocks/mock_aggsender_flow.go Updated mock to include new Signer() method

@goran-ethernal goran-ethernal force-pushed the feat/modify_poll_validator_committee branch 2 times, most recently from 2b8d723 to 80f000a Compare September 5, 2025 10:20
@goran-ethernal goran-ethernal force-pushed the feat/modify_poll_validator_committee branch from 5b36bee to a237623 Compare September 5, 2025 10:44
@goran-ethernal goran-ethernal marked this pull request as ready for review September 5, 2025 10:44
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 8, 2025

@goran-ethernal goran-ethernal merged commit 113d1af into feat/aggsender-multisig Sep 8, 2025
21 of 22 checks passed
@goran-ethernal goran-ethernal deleted the feat/modify_poll_validator_committee branch September 8, 2025 10:47
Stefan-Ethernal added a commit that referenced this pull request Oct 10, 2025
## 🔄 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]>
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.

4 participants