Sharding support #72
Sharding support #72
Conversation
Child mode (for child aggregator in sharded setup) differs from normal mode (for monolithic aggregator) by taking a shard ID and using its most significant bit as the "path" for the root node to get the corrrect hash value for the corresponding node of the parent aggregator's SMT.
Parent mode (for parent aggregator in sharded setup) differs from normal mode (for monolithic aggregator) in three ways: - is allows values in leaves to be updated; - the leaves are not hashed (instead they contain hash values computed by and received from child aggregators); - to maintain fixed structure, the tree is really not sparse at all.
Sharding child aggregator
- add ToBSON and FromBSON methods to all structs that are stored to mongo db - remove BSON marshalling from api.Timestamp struct - move some structs into separate files
…hard id part internally
Use mongo Date type to store timestamps
- join parent and child merkle paths in inclusion proof - request id shard bits are now managed internally in the SMT - set shardID type alias from int64 to int - use shardID type alias instead of int
Sharding: update child aggregator mode to latest smt changes
- store block number and leaf index as integers
Refactor: remove bson dependency from pkg/api Furthermore - store AggregatorRecord's BlockNumber and LeafIndex as integers - verify shard inclusion proof matches the previously sent hash
| ) | ||
|
|
||
| // Authenticator represents the authentication data for a commitment | ||
| type Authenticator struct { |
There was a problem hiding this comment.
why this (third) struct is needed? Seems api.Authenticator and AuthenticatorBSON to be enough
There was a problem hiding this comment.
We didn't add it, it was just moved to separate file. Looks like design choice to separate api and domain structs. Since it already exists, even though all the fields are the same, it's not worth refactoring IMO.
Sharding uses the rightmost bits so they do not conflict. |
Sharding readme
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive hierarchical sharding system for the aggregator, enabling parent and child aggregator modes. The changes include:
- Support for three sharding modes: standalone, parent, and child
- SMT (Sparse Merkle Tree) enhancements for sharded operations
- Parent-child proof joining and verification
- Removal of MongoDB BSON serialization in favor of explicit conversion
Reviewed Changes
Copilot reviewed 68 out of 70 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/sharding_e2e_test.go | E2E test suite validating hierarchical sharding with parent/child aggregators and proof joining |
| pkg/api/smt.go | Modified path verification to handle naked direction bits and joined proofs |
| pkg/api/types.go | Added ShardID type, shard-related request/response types, and removed BSON marshaling |
| internal/smt/smt.go | Major refactor adding child/parent SMT support with proof joining functionality |
| internal/service/parent_service.go | New parent aggregator service handling shard root submissions and proof generation |
| internal/sharding/root_aggregator_client.go | HTTP client for child aggregators to communicate with parent |
| internal/signing/commitment_validator.go | Added shard ID validation based on LSB matching |
| internal/storage/mongodb/*.go | Replaced direct BSON marshaling with explicit ToBSON/FromBSON conversions |
| internal/round/round_manager.go | Enhanced to support child mode round processing and parent communication |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // find position of MSB e.g. | ||
| // 0b111 -> BitLen=3 -> 3-1=2 | ||
| msbPos := shardBitmaskBigInt.BitLen() - 1 |
There was a problem hiding this comment.
The variable name 'msbPos' (Most Significant Bit Position) is misleading. According to the logic, this represents the position of the bit below the MSB, not the MSB itself. The MSB position would be at BitLen-1 before subtracting 1. Consider renaming to 'compareBitPos' or 'shardBitPos' to avoid confusion.
* create constant for miniBatchSize
The implementation of sharded SMT did not have the bug the spec had (and that was fixed in unicitynetwork/specs#4), but the old tests would not have caught it if it did.
Add optional parent and child modes for aggregator for sharding