Assembler synchronizer test#963
Open
DorKatzelnick wants to merge 4 commits into
Open
Conversation
DorKatzelnick
force-pushed
the
assemblerSyncTest
branch
from
79c58d7 to
a448018
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>
Signed-off-by: Dor.Katzelnick <Dor.Katzelnick@ibm.com>
tock-ibm
force-pushed
the
assemblerSyncTest
branch
from
a448018 to
4b3f2f2
Compare
tock-ibm
reviewed
Jul 6, 2026
| require.Eventually(t, func() bool { | ||
| return joiningAsm.GetTxCount() == expectedTxCountAfterConfig | ||
| }, 30*time.Second, 100*time.Millisecond, | ||
| "joining assembler should sync all blocks including config block") |
Contributor
There was a problem hiding this comment.
send a couple of decisions and batches after join had finished to verify it continues normally
| require.Eventually(t, func() bool { | ||
| return joiningAsm.GetTxCount() == expectedTxCount | ||
| }, 30*time.Second, 100*time.Millisecond, | ||
| "joining assembler should sync genesis block and config block") |
Contributor
There was a problem hiding this comment.
send a couple of decisions and batches after join had finished to verify it continues normally
| require.Eventually(t, func() bool { | ||
| return joiningAsm.GetTxCount() == expectedTxCountAfterConfig | ||
| }, 60*time.Second, 100*time.Millisecond, | ||
| "joining assembler should sync despite one unavailable source") |
Contributor
There was a problem hiding this comment.
send a couple of decisions and batches after join had finished to verify it continues normally
Contributor
There was a problem hiding this comment.
There is a lot of repeating code in these 3 test cases (starting the network etc).
Consider a struct sync_test_setup that does it and keeps all the moving parts.
Contributor
There was a problem hiding this comment.
Test the case where there also config blocks that are sync'ed. That is: Genesis - data blocks - config block - data blocks - join block
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an assembler BFT synchronizer path to allow an assembler with an incomplete/empty ledger to fetch missing blocks (including genesis/config blocks) from existing assemblers before starting, and adds integration-style tests that exercise real sync during a membership change.
Changes:
- Add a new
node/assembler/synchronizerpackage that builds and drives a BFT block deliverer into a localSyncBuffer. - Extend assembler initialization to sync up to a target config block height when the supplied config block is ahead of local ledger height.
- Add tests/stubs to validate joining behavior (including “no regular blocks” and “some sources unavailable” scenarios).
Reviewed changes
Copilot reviewed 15 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| node/config/config.go | Adds AssemblerInfo struct for sharing assembler endpoint/TLS info. |
| node/assembler/utils.go | Adds helper to extract gRPC client TLS config for assembler-to-assembler dialing. |
| node/assembler/synchronizer/util.go | Adds adapters/factories (ledger adapter, deliverer factory, endpoints extractor). |
| node/assembler/synchronizer/synchronizer_factory.go | Adds factory wiring for creating the assembler synchronizer. |
| node/assembler/synchronizer/synchronizer_bft.go | Implements main sync logic (genesis fetch + block delivery into buffer). |
| node/assembler/synchronizer/sync_buffer.go | Adds buffered block handoff mechanism between deliverer and ledger writer. |
| node/assembler/synchronizer/genesis_fetcher_factory.go | Creates a block puller used to fetch genesis blocks from endpoints. |
| node/assembler/synchronizer/block_verifier.go | Adds verifier factory interface + noop verifier implementation. |
| node/assembler/synchronizer/mocks/verifier_factory.go | Counterfeiter mock for verifier factory. |
| node/assembler/synchronizer/mocks/synchronizer_with_stop.go | Counterfeiter mock for synchronizer interface. |
| node/assembler/synchronizer/mocks/synchronizer_factory.go | Counterfeiter mock for synchronizer factory. |
| node/assembler/synchronizer/mocks/genesis_fetcher.go | Counterfeiter mock for genesis fetcher. |
| node/assembler/synchronizer/mocks/genesis_fetcher_factory.go | Counterfeiter mock for genesis fetcher factory. |
| node/assembler/synchronizer/mocks/bft_deliverer_factory.go | Counterfeiter mock for BFT deliverer factory. |
| node/assembler/synchronizer/mocks/bft_block_deliverer.go | Counterfeiter mock for BFT block deliverer. |
| node/assembler/synchronizer/mocks/assembler_support.go | Counterfeiter mock for assembler support adapter. |
| node/assembler/stub_consenter_test.go | Extends test stub consenter to emit config decisions. |
| node/assembler/oba_utils_test.go | Adds OBA creator seeded from a real genesis block hash chain. |
| node/assembler/assembler.go | Updates ledger init to sync missing blocks and adds synchronizer factory injection. |
| node/assembler/assembler_test.go | Updates unit tests to account for new synchronizer dependency/behavior. |
| node/assembler/assembler_sync_test.go | Adds end-to-end style tests for real synchronizer behavior. |
| node/assembler/assembler_sync_support.go | Extends support adapter with ClientConfig() for synchronizer dialing. |
| config/config.go | Adds assembler extraction + address extraction from consensus metadata. |
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
+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
+160
to
+166
| bootConfigBlock := a.BootConfigBlock | ||
|
|
||
| a.Logger.Infof("Creating BFTDeliverer, last Block in ledger: %d, last config block in ledger: %d, boot config block: %d", lastBlock.Header.Number, ledgerLastConfigBlock.Header.Number, bootConfigBlock.Header.Number) | ||
|
|
||
| blockOps := &utils.CommonConfigBlockOperations{} | ||
| lastConfigEnv, err := blockOps.ConfigFromBlock(bootConfigBlock) | ||
| if err != nil { |
Comment on lines
+176
to
+182
| clientConfig := a.ClusterDialer.Config // The cluster and block puller use slightly different options | ||
| clientConfig.AsyncConnect = false | ||
| clientConfig.SecOpts.VerifyCertificate = nil | ||
|
|
||
| block, _ := pem.Decode(clientConfig.SecOpts.Certificate) | ||
| tlsCertHash := util.ComputeSHA256(block.Bytes) | ||
|
|
Comment on lines
+92
to
+94
| BlockPullerFactory: &GenesisFetcherCreator{}, | ||
| VerifierFactory: &noopVerifierCreator{}, // TODO replace with real verifer | ||
| BFTDelivererFactory: &bftDelivererCreator{}, |
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.