4364: Test with blocks which have early duplicates which are fully sp…#198
Conversation
|
🤖 Claude Code Review Status: Complete Findings: The PR fixes a critical bug in duplicate transaction detection where the index calculation was incorrect for incomplete last subtrees. The fix is correct, but the tests have several issues that need addressing per reviewer feedback: Test Issues (per human reviewer comments):
Core Fix (model/Block.go:629-635): Action Items: |
| err := td.BlockchainClient.Run(td.Ctx, "test") | ||
| require.NoError(t, err) | ||
|
|
||
| err = td.BlockAssemblyClient.GenerateBlocks(td.Ctx, &blockassembly_api.GenerateBlocksRequest{Count: 101}) |
There was a problem hiding this comment.
we have MineAndWait helper, may be use that
also 101 is too heavy for CI, 2 should be good for tests
| err = td.BlockValidationClient.ProcessBlock(td.Ctx, block102, block102.Height, "", "legacy") | ||
| require.Error(t, err, "Block with duplicates should be rejected even with concurrency=0") | ||
|
|
||
| t.Logf("✓ Duplicate detection works with concurrency=0 (uses default)") |
There was a problem hiding this comment.
we need to avoid this pattern of t.Logf, unless required
| // NOTE: This test is skipped because with spending transactions in the block, the block validation | ||
| // fails during transaction processing (UTXO validation) before the duplicate detection check runs. | ||
| // The test TestEarlyDuplicatePartiallySpentAndPruned covers a similar scenario successfully. | ||
| func TestEarlyDuplicateFullySpentAndPruned(t *testing.T) { |
| require.NoError(t, err) | ||
|
|
||
| err = td.BlockAssemblyClient.GenerateBlocks(td.Ctx, &blockassembly_api.GenerateBlocksRequest{Count: 101}) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
Generate blocks create blocks with just coinbase, so it gets tested by default, this test can be removed
…ent & pruned