Assembler synchronizer integration#962
Open
DorKatzelnick wants to merge 3 commits into
Open
Conversation
109643b to
3e4a27d
Compare
Signed-off-by: Dor.Katzelnick <Dor.Katzelnick@ibm.com>
Signed-off-by: Dor.Katzelnick <Dor.Katzelnick@ibm.com>
Signed-off-by: Dor.Katzelnick <Dor.Katzelnick@ibm.com>
tock-ibm
reviewed
Jul 6, 2026
| a.logger.Infof("Appended genesis block, header digest: %s", hex.EncodeToString(protoutil.BlockHeaderHash(configBlock.GetHeader()))) | ||
| } else if configBlockNumber > ledgerHeight { | ||
| // genesis block is block number 0, so if config block number is higher than ledger height, it means the assembler needs to sync to the config block before starting. | ||
| a.logger.Warnf("Config block number %d is higher than current ledger height %d, assembler will need to sync to the config block", configBlockNumber, ledgerHeight) |
| a.logger.Panicf("error while syncing to config block %v", err) | ||
| } | ||
| } else { | ||
| // todo - should panic? |
Contributor
There was a problem hiding this comment.
Not panic - this is the normal case when restarting after boot or join
Contributor
There was a problem hiding this comment.
Pull request overview
This PR integrates an “assembler synchronizer” path that can bootstrap/sync an assembler’s ledger to a target config block height by fetching genesis/blocks over BFT deliver, and adds config helpers for assembler endpoint extraction and TLS client configuration.
Changes:
- Add new
node/assembler/synchronizerpackage (BFT deliverer wiring, genesis fetching, buffering, verifier factory abstraction). - Integrate synchronizer invocation into assembler ledger initialization when the config block number is ahead of local ledger height.
- Extend config extraction to include assembler endpoint info and assembler address extraction from channel config.
Reviewed changes
Copilot reviewed 12 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| node/config/config.go | Adds AssemblerInfo struct for assembler endpoint/TLS metadata. |
| node/assembler/utils.go | Adds TLS comm.ClientConfig extraction for assembler-to-assembler dialing. |
| node/assembler/synchronizer/util.go | Adds deliverer factory/adapter types and assembler endpoints extractor. |
| node/assembler/synchronizer/synchronizer_factory.go | Adds synchronizer factory API and policy-based creator. |
| node/assembler/synchronizer/synchronizer_bft.go | Implements BFT-based sync flow, deliverer creation, and genesis matching logic. |
| node/assembler/synchronizer/sync_buffer.go | Adds buffering between deliverer and ledger writer. |
| node/assembler/synchronizer/genesis_fetcher_factory.go | Creates a block puller used to retrieve candidate genesis blocks/endpoints. |
| node/assembler/synchronizer/block_verifier.go | Introduces verifier factory interface plus a noop verifier implementation. |
| node/assembler/synchronizer/mocks/verifier_factory.go | Generated mock for VerifierFactory. |
| node/assembler/synchronizer/mocks/synchronizer_with_stop.go | Generated mock for SynchronizerWithStop. |
| node/assembler/synchronizer/mocks/synchronizer_factory.go | Generated mock for SynchronizerFactory. |
| node/assembler/synchronizer/mocks/genesis_fetcher.go | Generated mock for GenesisFetcher. |
| node/assembler/synchronizer/mocks/genesis_fetcher_factory.go | Generated mock for GenesisFetcherFactory. |
| node/assembler/synchronizer/mocks/bft_deliverer_factory.go | Generated mock for BFTDelivererFactory. |
| node/assembler/synchronizer/mocks/bft_block_deliverer.go | Generated mock for BFTBlockDeliverer. |
| node/assembler/synchronizer/mocks/assembler_support.go | Generated mock for AssemblerSupport. |
| node/assembler/assembler.go | Wires synchronizer into ledger init; adds locking to GetTxCount; updates init/metrics path. |
| node/assembler/assembler_test.go | Introduces synchronizer factory mock plumbing and adjusts non-genesis config-block test. |
| node/assembler/assembler_sync_support.go | Exposes ClientConfig() via AssemblerSupportAdapter for synchronizer dialing. |
| config/config.go | Adds assembler extraction and ExtractAssemblerAddresses for channel-config endpoint resolution. |
Files not reviewed (8)
- node/assembler/synchronizer/mocks/assembler_support.go: Generated file
- node/assembler/synchronizer/mocks/bft_block_deliverer.go: Generated file
- node/assembler/synchronizer/mocks/bft_deliverer_factory.go: Generated file
- node/assembler/synchronizer/mocks/genesis_fetcher.go: Generated file
- node/assembler/synchronizer/mocks/genesis_fetcher_factory.go: Generated file
- node/assembler/synchronizer/mocks/synchronizer_factory.go: Generated file
- node/assembler/synchronizer/mocks/synchronizer_with_stop.go: Generated file
- node/assembler/synchronizer/mocks/verifier_factory.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+180
to
+182
| block, _ := pem.Decode(clientConfig.SecOpts.Certificate) | ||
| tlsCertHash := util.ComputeSHA256(block.Bytes) | ||
|
|
Comment on lines
+254
to
+256
| } else if configBlockNumber > ledgerHeight { | ||
| // genesis block is block number 0, so if config block number is higher than ledger height, it means the assembler needs to sync to the config block before starting. | ||
| a.logger.Warnf("Config block number %d is higher than current ledger height %d, assembler will need to sync to the config block", configBlockNumber, ledgerHeight) |
Comment on lines
+85
to
+95
| return &AssemblerBFTSynchronizer{ | ||
| SelfPartyID: selfID, | ||
| TargetHeight: targetHeight, | ||
| Support: support, | ||
| CryptoProvider: bccsp, | ||
| ClusterDialer: &comm.PredicateDialer{Config: support.ClientConfig()}, | ||
| LocalConfigCluster: localConfigCluster, | ||
| BlockPullerFactory: &GenesisFetcherCreator{}, | ||
| VerifierFactory: &noopVerifierCreator{}, // TODO replace with real verifer | ||
| BFTDelivererFactory: &bftDelivererCreator{}, | ||
| Logger: logger, |
Comment on lines
+286
to
289
| // Act & Assert: starting with a non-genesis config block and an empty ledger should panic | ||
| // because the assembler cannot operate without first syncing the missing blocks. | ||
| require.Panics(t, func() { test.StartAssembler() }) | ||
| } |
Comment on lines
+75
to
+90
| func (a *AssemblerBFTSynchronizer) synchronize() error { | ||
| startHeight := a.Support.Height() | ||
|
|
||
| if startHeight > a.TargetHeight { | ||
| return fmt.Errorf("error synchronizing assembler: startHeight %d is greater than targetHeight %d", startHeight, a.TargetHeight) | ||
| } | ||
|
|
||
| if startHeight == 0 { | ||
| genesisBlock, err := a.fetchGenesisBlock() | ||
| if err != nil { | ||
| a.Logger.Panicf("Cannot join the cluster: %s", errors.Wrap(err, "failed to fetch genesis block")) | ||
| } | ||
| a.Support.WriteConfigBlock(genesisBlock) | ||
| startHeight = a.Support.Height() | ||
| a.Logger.Infof("Fetched and wrote genesis block, new height: %d, party: %d", startHeight, a.SelfPartyID) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.