From a0e0d8d8da9a565bf4f513293d362ab8ddde7889 Mon Sep 17 00:00:00 2001 From: Alexander Hentschel Date: Tue, 4 Feb 2025 14:16:46 -0800 Subject: [PATCH 1/2] documented storage abstraction `storage.ResultApprovals` --- storage/approvals.go | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/storage/approvals.go b/storage/approvals.go index bfd77dadb47..e1e02f0be8d 100644 --- a/storage/approvals.go +++ b/storage/approvals.go @@ -4,17 +4,39 @@ import ( "github.com/onflow/flow-go/model/flow" ) +// ResultApprovals implements persistent storage for result approvals. +// Implementations of this interface must be concurrency safe. +// +// CAUTION suitable only for _Verification Nodes_ for persisting their _own_ approvals! +// - In general, the Flow protocol requires multiple approvals for the same chunk from different +// verification nodes. In other words, there are multiple different approvals for the same chunk. +// - Internally, ResultApprovals populates an index from Executed Chunk ➜ ResultApproval. This is +// *only safe* for Verification Nodes when tracking their own approvals (for the same ExecutionResult, +// a Verifier will always produce the same approval) type ResultApprovals interface { - // Store stores a ResultApproval + // Store stores a ResultApproval by its ID. Store(result *flow.ResultApproval) error - // Index indexes a ResultApproval by result ID and chunk index + // Index indexes a ResultApproval by result ID and chunk index. + // + // CAUTION: the Flow protocol requires multiple approvals for the same chunk from different verification + // nodes. In other words, there are multiple different approvals for the same chunk. Therefore, the index + // Executed Chunk ➜ ResultApproval ID (populated here) is *only safe* to be used by Verification Nodes + // for tracking their own approvals. + // + // For the same ExecutionResult, a Verifier will always produce the same approval. Therefore, this operation + // is idempotent, i.e. repeated calls with the *same inputs* are equivalent to just calling the method once; + // still the method succeeds on each call. However, when attempting to index *different* ResultApproval IDs + // for the same key (resultID, chunkIndex) this method returns an exception, as this should never happen for + // a correct Verification Node indexing its own approvals. Index(resultID flow.Identifier, chunkIndex uint64, approvalID flow.Identifier) error - // ByID retrieves a ResultApproval by its ID + // ByID retrieves a ResultApproval by its ID. + // Returns `storage.ErrNotFound` if no Approval with the given ID has been stored. ByID(approvalID flow.Identifier) (*flow.ResultApproval, error) - // ByChunk retrieves a ResultApproval by result ID and chunk index + // ByChunk retrieves a ResultApproval by result ID and chunk index. + // Returns `storage.ErrNotFound` if no Approval for the given key (resultID, chunkIndex) has been stored. ByChunk(resultID flow.Identifier, chunkIndex uint64) (*flow.ResultApproval, error) } From c80c420c131bb3eab74ac67784f8f23424afc17a Mon Sep 17 00:00:00 2001 From: Alexander Hentschel Date: Tue, 18 Feb 2025 14:48:37 -0800 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Yurii Oleksyshyn --- storage/approvals.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/storage/approvals.go b/storage/approvals.go index e1e02f0be8d..7b8d29f0664 100644 --- a/storage/approvals.go +++ b/storage/approvals.go @@ -16,6 +16,7 @@ import ( type ResultApprovals interface { // Store stores a ResultApproval by its ID. + // No errors are expected during normal operations. Store(result *flow.ResultApproval) error // Index indexes a ResultApproval by result ID and chunk index. @@ -30,13 +31,14 @@ type ResultApprovals interface { // still the method succeeds on each call. However, when attempting to index *different* ResultApproval IDs // for the same key (resultID, chunkIndex) this method returns an exception, as this should never happen for // a correct Verification Node indexing its own approvals. + // No errors are expected during normal operations. Index(resultID flow.Identifier, chunkIndex uint64, approvalID flow.Identifier) error // ByID retrieves a ResultApproval by its ID. - // Returns `storage.ErrNotFound` if no Approval with the given ID has been stored. + // Returns [storage.ErrNotFound] if no Approval with the given ID has been stored. ByID(approvalID flow.Identifier) (*flow.ResultApproval, error) // ByChunk retrieves a ResultApproval by result ID and chunk index. - // Returns `storage.ErrNotFound` if no Approval for the given key (resultID, chunkIndex) has been stored. + // Returns [storage.ErrNotFound] if no Approval for the given key (resultID, chunkIndex) has been stored. ByChunk(resultID flow.Identifier, chunkIndex uint64) (*flow.ResultApproval, error) }