diff --git a/engine/consensus/matching/engine_test.go b/engine/consensus/matching/engine_test.go index 45d61a5387d..37f379fd8af 100644 --- a/engine/consensus/matching/engine_test.go +++ b/engine/consensus/matching/engine_test.go @@ -651,7 +651,7 @@ func (ms *MatchingSuite) TestSealResultInvalidChunks() { ms.sealedResults[previous.ID()] = previous // add an extra chunk - chunk := unittest.ChunkFixture() + chunk := unittest.ChunkFixture(block.ID()) chunk.Index = uint64(len(block.Payload.Guarantees)) result.Chunks = append(result.Chunks, chunk) diff --git a/engine/execution/ingestion/engine.go b/engine/execution/ingestion/engine.go index da024c71c5a..934a231cc08 100644 --- a/engine/execution/ingestion/engine.go +++ b/engine/execution/ingestion/engine.go @@ -694,8 +694,9 @@ func (e *Engine) saveExecutionResults( defer span.Finish() originalState := startState + blockID := executableBlock.ID() - err := e.execState.PersistStateInteractions(childCtx, executableBlock.ID(), stateInteractions) + err := e.execState.PersistStateInteractions(childCtx, blockID, stateInteractions) if err != nil && !errors.Is(err, storage.ErrAlreadyExists) { return nil, err } @@ -724,7 +725,7 @@ func (e *Engine) saveExecutionResults( collectionID = flow.ZeroID } - chunk := generateChunk(i, startState, endState, collectionID) + chunk := generateChunk(i, startState, endState, collectionID, blockID) // chunkDataPack allRegisters := view.AllRegisters() @@ -732,7 +733,7 @@ func (e *Engine) saveExecutionResults( values, proofs, err := e.execState.GetRegistersWithProofs(childCtx, chunk.StartState, allRegisters) if err != nil { return nil, fmt.Errorf( - "error reading registers with proofs for chunk number [%v] of block [%x] ", i, executableBlock.ID(), + "error reading registers with proofs for chunk number [%v] of block [%x] ", i, blockID, ) } @@ -748,7 +749,7 @@ func (e *Engine) saveExecutionResults( startState = endState } - err = e.execState.PersistStateCommitment(childCtx, executableBlock.ID(), endState) + err = e.execState.PersistStateCommitment(childCtx, blockID, endState) if err != nil { return nil, fmt.Errorf("failed to store state commitment: %w", err) } @@ -774,7 +775,6 @@ func (e *Engine) saveExecutionResults( span, _ := e.tracer.StartSpanFromContext(childCtx, trace.EXESaveTransactionEvents) defer span.Finish() - blockID := executableBlock.ID() if len(events) > 0 { err = e.events.Store(blockID, events) if err != nil { @@ -790,7 +790,7 @@ func (e *Engine) saveExecutionResults( err = func() error { span, _ := e.tracer.StartSpanFromContext(childCtx, trace.EXESaveTransactionResults) defer span.Finish() - blockID := executableBlock.ID() + err = e.transactionResults.BatchStore(blockID, txResults) if err != nil { return fmt.Errorf("failed to store transaction result error: %w", err) @@ -841,7 +841,9 @@ func (e *Engine) logExecutableBlock(eb *entity.ExecutableBlock) { } // generateChunk creates a chunk from the provided computation data. -func generateChunk(colIndex int, startState, endState flow.StateCommitment, colID flow.Identifier) *flow.Chunk { +func generateChunk(colIndex int, + startState, endState flow.StateCommitment, + colID, blockID flow.Identifier) *flow.Chunk { return &flow.Chunk{ ChunkBody: flow.ChunkBody{ CollectionIndex: uint(colIndex), @@ -850,6 +852,7 @@ func generateChunk(colIndex int, startState, endState flow.StateCommitment, colI // Otherwise, the chances of there being chunks with the same ID before all these TODOs are done is large, since // startState stays the same if blocks are empty EventCollection: colID, + BlockID: blockID, // TODO: record gas used TotalComputationUsed: 0, // TODO: record number of txs diff --git a/engine/verification/match/engine_test.go b/engine/verification/match/engine_test.go index 811f064a166..7863066dbc9 100644 --- a/engine/verification/match/engine_test.go +++ b/engine/verification/match/engine_test.go @@ -812,6 +812,7 @@ func ChunkWithIndex(blockID flow.Identifier, index int) *flow.Chunk { ChunkBody: flow.ChunkBody{ CollectionIndex: uint(index), EventCollection: blockID, // ensure chunks from different blocks with the same index will have different chunk ID + BlockID: blockID, }, } return chunk diff --git a/engine/verification/utils/fixtures.go b/engine/verification/utils/fixtures.go index ee5902ff232..282692e9b07 100644 --- a/engine/verification/utils/fixtures.go +++ b/engine/verification/utils/fixtures.go @@ -171,50 +171,55 @@ func CompleteExecutionResultFixture(t *testing.T, chunkCount int, chain flow.Cha // everything is wired properly, but with the minimum viable content provided. This version is basically used // for profiling. func LightExecutionResultFixture(chunkCount int) CompleteExecutionResult { - chunks := make([]*flow.Chunk, 0) collections := make([]*flow.Collection, 0, chunkCount) guarantees := make([]*flow.CollectionGuarantee, 0, chunkCount) chunkDataPacks := make([]*flow.ChunkDataPack, 0, chunkCount) + // creates collections and guarantees for i := 0; i < chunkCount; i++ { - // creates one guaranteed collection per chunk coll := unittest.CollectionFixture(1) guarantee := coll.Guarantee() collections = append(collections, &coll) guarantees = append(guarantees, &guarantee) + } + + payload := flow.Payload{ + Guarantees: guarantees, + } + + header := unittest.BlockHeaderFixture() + header.Height = 0 + header.PayloadHash = payload.Hash() + + block := flow.Block{ + Header: &header, + Payload: &payload, + } + blockID := block.ID() + // creates chunks + chunks := make([]*flow.Chunk, 0) + for i := 0; i < chunkCount; i++ { chunk := &flow.Chunk{ ChunkBody: flow.ChunkBody{ CollectionIndex: uint(i), + BlockID: blockID, EventCollection: unittest.IdentifierFixture(), }, Index: uint64(i), } chunks = append(chunks, chunk) - // creates a chunk data pack for the chunk + // creates a light (quite empty) chunk data pack for the chunk at bare minimum chunkDataPack := flow.ChunkDataPack{ ChunkID: chunk.ID(), } chunkDataPacks = append(chunkDataPacks, &chunkDataPack) } - payload := flow.Payload{ - Guarantees: guarantees, - } - - header := unittest.BlockHeaderFixture() - header.Height = 0 - header.PayloadHash = payload.Hash() - - block := flow.Block{ - Header: &header, - Payload: &payload, - } - result := flow.ExecutionResult{ ExecutionResultBody: flow.ExecutionResultBody{ - BlockID: block.ID(), + BlockID: blockID, Chunks: chunks, }, } @@ -292,6 +297,7 @@ func executeCollection( StartState: startStateCommitment, // TODO: include event collection hash EventCollection: flow.ZeroID, + BlockID: executableBlock.ID(), // TODO: record gas used TotalComputationUsed: 0, // TODO: record number of txs diff --git a/go.mod b/go.mod index 292c8510a66..305506e0362 100644 --- a/go.mod +++ b/go.mod @@ -41,6 +41,7 @@ require ( github.com/spf13/cobra v0.0.6 github.com/spf13/pflag v1.0.3 github.com/spf13/viper v1.4.0 + github.com/stretchr/objx v0.2.0 // indirect github.com/stretchr/testify v1.6.1 github.com/uber/jaeger-client-go v2.22.1+incompatible github.com/uber/jaeger-lib v2.3.0+incompatible // indirect diff --git a/go.sum b/go.sum index 0b0b15c877f..872823a0863 100644 --- a/go.sum +++ b/go.sum @@ -799,6 +799,8 @@ github.com/steakknife/hamming v0.0.0-20180906055917-c99c65617cd3/go.mod h1:hpGUW github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1 h1:2vfRuCMp5sSVIDSqO8oNnWJq7mPa6KVP3iPIwFBuy8A= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48= +github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= diff --git a/integration/tests/consensus/sealing_test.go b/integration/tests/consensus/sealing_test.go index 22663f9e35a..ee8e1f10c7f 100644 --- a/integration/tests/consensus/sealing_test.go +++ b/integration/tests/consensus/sealing_test.go @@ -180,8 +180,9 @@ SearchLoop: CollectionIndex: 0, // irrelevant for consensus node StartState: nil, // irrelevant for consensus node EventCollection: flow.ZeroID, // irrelevant for consensus node - TotalComputationUsed: 0, // irrelevant for consensus node - NumberOfTransactions: 0, // irrelevant for consensus node + BlockID: targetID, + TotalComputationUsed: 0, // irrelevant for consensus node + NumberOfTransactions: 0, // irrelevant for consensus node }, Index: 0, // should start at zero EndState: unittest.StateCommitmentFixture(), // random end execution state diff --git a/integration/tests/verification/system_chunk_test.go b/integration/tests/verification/system_chunk_test.go new file mode 100644 index 00000000000..92bdeb4dd40 --- /dev/null +++ b/integration/tests/verification/system_chunk_test.go @@ -0,0 +1,51 @@ +package verification + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +func TestSystemChunk(t *testing.T) { + suite.Run(t, new(SystemChunkTestSuite)) +} + +type SystemChunkTestSuite struct { + Suite +} + +// TestSystemChunkIDsShouldBeDifferent evaluates that system chunk of consecutive blocks that +// do not cause state change have different chunk Ids. +func (st *SystemChunkTestSuite) TestSystemChunkIDsShouldBeDifferent() { + // waits for first finalized block, called blockA. + blockA := st.BlockState.WaitForFirstFinalized(st.T()) + st.T().Logf("blockA generated, height: %v ID: %v", blockA.Header.Height, blockA.Header.ID()) + + // waits for the next finalized block after blockA, called blockB. + blockB := st.BlockState.WaitForFinalizedChild(st.T(), blockA) + st.T().Logf("blockB generated, height: %v ID: %v", blockB.Header.Height, blockB.Header.ID()) + + // waits for execution receipt for blockA from execution node, called receiptA. + receiptA := st.ReceiptState.WaitForReceiptFrom(st.T(), blockA.Header.ID(), st.exeID) + resultAId := receiptA.ExecutionResult.ID() + st.T().Logf("receipt for blockA generated: result ID: %x", resultAId) + + // waits for execution receipt for blockB from execution node, called receiptB. + receiptB := st.ReceiptState.WaitForReceiptFrom(st.T(), blockB.Header.ID(), st.exeID) + resultBId := receiptB.ExecutionResult.ID() + st.T().Logf("receipt for blockB generated: result ID: %x", resultBId) + + // Todo: drop this part once system chunk changes the state + // requires that execution state is not changed between block A and B + stateA := receiptA.ExecutionResult.FinalStateCommit + stateB := receiptB.ExecutionResult.FinalStateCommit + require.Equal(st.T(), stateA, stateB) + + // computes ids of system chunk for result A and B + systemChunkAId := receiptA.ExecutionResult.Chunks[0].ID() + systemChunkBId := receiptB.ExecutionResult.Chunks[0].ID() + + // requires that system chunk Id of execution results be different + require.NotEqual(st.T(), systemChunkAId, systemChunkBId) +} diff --git a/model/flow/chunk.go b/model/flow/chunk.go index ff1884e2470..02592edc34f 100644 --- a/model/flow/chunk.go +++ b/model/flow/chunk.go @@ -6,6 +6,7 @@ type ChunkBody struct { // execution info StartState StateCommitment // start state when starting executing this chunk EventCollection Identifier // Events generated by executing results + BlockID Identifier // Block id of the execution result this chunk belongs to // Computation consumption info TotalComputationUsed uint64 // total amount of computation used by running all txs in this chunk diff --git a/model/flow/chunk_test.go b/model/flow/chunk_test.go index 48ecb008aca..ee9425ac74a 100644 --- a/model/flow/chunk_test.go +++ b/model/flow/chunk_test.go @@ -1,16 +1,19 @@ -package flow +package flow_test import ( "testing" "github.com/stretchr/testify/require" + + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" ) // TestChunkList_ByIndex evaluates reliability of ByIndex method against within and // out of range indices func TestChunkList_ByIndex(t *testing.T) { // creates a chunk list with the size of 10 - var chunkList ChunkList = make([]*Chunk, 10) + var chunkList flow.ChunkList = make([]*flow.Chunk, 10) // an out of index chunk by index _, ok := chunkList.ByIndex(11) @@ -19,5 +22,56 @@ func TestChunkList_ByIndex(t *testing.T) { // a within range chunk by index _, ok = chunkList.ByIndex(1) require.True(t, ok) +} + +// TestDistinctChunkIDs_EmptyChunks evaluates that two empty chunks +// with the distinct block ids would have distinct chunk ids. +func TestDistinctChunkIDs_EmptyChunks(t *testing.T) { + // generates two random block ids and requires them + // being distinct + blockIdA := unittest.IdentifierFixture() + blockIdB := unittest.IdentifierFixture() + require.NotEqual(t, blockIdA, blockIdB) + + // generates a chunk associated with each block id + chunkA := &flow.Chunk{ + ChunkBody: flow.ChunkBody{ + BlockID: blockIdA, + }, + } + + chunkB := &flow.Chunk{ + ChunkBody: flow.ChunkBody{ + BlockID: blockIdB, + }, + } + + require.NotEqual(t, chunkA.ID(), chunkB.ID()) +} + +// TestDistinctChunkIDs_FullChunks evaluates that two full chunks +// with completely identical fields but distinct block ids have +// distinct chunk ids. +func TestDistinctChunkIDs_FullChunks(t *testing.T) { + // generates two random block ids and requires them + // being distinct + blockIdA := unittest.IdentifierFixture() + blockIdB := unittest.IdentifierFixture() + require.NotEqual(t, blockIdA, blockIdB) + + // generates a chunk associated with blockA + chunkA := unittest.ChunkFixture(blockIdA) + + // generates a deep copy of chunkA in chunkB + chunkB := *chunkA + + // since chunkB is a deep copy of chunkA their + // chunk ids should be the same + require.Equal(t, chunkA.ID(), chunkB.ID()) + + // changes block id in chunkB + chunkB.BlockID = blockIdB + // chunks with distinct block ids should have distinct chunk ids + require.NotEqual(t, chunkA.ID(), chunkB.ID()) } diff --git a/module/chunks/chunkVerifier_test.go b/module/chunks/chunkVerifier_test.go index 698e48a2808..5bb01955a13 100644 --- a/module/chunks/chunkVerifier_test.go +++ b/module/chunks/chunkVerifier_test.go @@ -163,6 +163,7 @@ func GetBaselineVerifiableChunk(t *testing.T, script []byte) *verification.Verif Header: &header, Payload: &payload, } + blockID := block.ID() // registerTouch and State setup id1 := state.RegisterID(string(make([]byte, 32)), "", "") @@ -197,6 +198,7 @@ func GetBaselineVerifiableChunk(t *testing.T, script []byte) *verification.Verif ChunkBody: flow.ChunkBody{ CollectionIndex: 0, StartState: startState, + BlockID: blockID, }, Index: 0, } @@ -210,7 +212,7 @@ func GetBaselineVerifiableChunk(t *testing.T, script []byte) *verification.Verif // ExecutionResult setup result := flow.ExecutionResult{ ExecutionResultBody: flow.ExecutionResultBody{ - BlockID: block.ID(), + BlockID: blockID, Chunks: flow.ChunkList{&chunk}, }, } diff --git a/module/chunks/publicAssign_test.go b/module/chunks/publicAssign_test.go index 1901ecbc144..788afbbe5ef 100644 --- a/module/chunks/publicAssign_test.go +++ b/module/chunks/publicAssign_test.go @@ -336,11 +336,14 @@ func (a *PublicAssignmentTestSuite) CreateChunks(num int, t *testing.T) flow.Chu _, err := rand.Read(state) require.NoError(t, err) + blockID := unittest.IdentifierFixture() + // creates chunk c := &flow.Chunk{ Index: uint64(i), ChunkBody: flow.ChunkBody{ StartState: state, + BlockID: blockID, }, } list.Insert(c) diff --git a/utils/unittest/fixtures.go b/utils/unittest/fixtures.go index 3e17f70a050..1848d28c09f 100644 --- a/utils/unittest/fixtures.go +++ b/utils/unittest/fixtures.go @@ -359,7 +359,7 @@ func ResultForBlockFixture(block *flow.Block) *flow.ExecutionResult { PreviousResultID: IdentifierFixture(), BlockID: block.Header.ID(), FinalStateCommit: StateCommitmentFixture(), - Chunks: ChunksFixture(uint(chunks)), + Chunks: ChunksFixture(uint(chunks), block.ID()), }, Signatures: SignaturesFixture(6), } @@ -375,14 +375,15 @@ func ExecutionReceiptFixture() *flow.ExecutionReceipt { } func ExecutionResultFixture() *flow.ExecutionResult { + blockID := IdentifierFixture() return &flow.ExecutionResult{ ExecutionResultBody: flow.ExecutionResultBody{ PreviousResultID: IdentifierFixture(), - BlockID: IdentifierFixture(), + BlockID: blockID, FinalStateCommit: StateCommitmentFixture(), Chunks: flow.ChunkList{ - ChunkFixture(), - ChunkFixture(), + ChunkFixture(blockID), + ChunkFixture(blockID), }, }, Signatures: SignaturesFixture(6), @@ -564,7 +565,7 @@ func IdentityListFixture(n int, opts ...func(*flow.Identity)) flow.IdentityList return identities } -func ChunkFixture() *flow.Chunk { +func ChunkFixture(blockID flow.Identifier) *flow.Chunk { return &flow.Chunk{ ChunkBody: flow.ChunkBody{ CollectionIndex: 42, @@ -572,16 +573,17 @@ func ChunkFixture() *flow.Chunk { EventCollection: IdentifierFixture(), TotalComputationUsed: 4200, NumberOfTransactions: 42, + BlockID: blockID, }, Index: 0, EndState: StateCommitmentFixture(), } } -func ChunksFixture(n uint) []*flow.Chunk { +func ChunksFixture(n uint, blockID flow.Identifier) []*flow.Chunk { chunks := make([]*flow.Chunk, 0, n) for i := uint64(0); i < uint64(n); i++ { - chunk := ChunkFixture() + chunk := ChunkFixture(blockID) chunk.Index = i chunks = append(chunks, chunk) } @@ -687,6 +689,7 @@ func VerifiableChunkDataFixture(chunkIndex uint64) *verification.VerifiableChunk ChunkBody: flow.ChunkBody{ CollectionIndex: uint(i), StartState: StateCommitmentFixture(), + BlockID: block.ID(), }, Index: uint64(i), }