Skip to content

Commit 9767bbf

Browse files
authored
Merge pull request #7806 from onflow/leo/enable-view-index
[Storage] Enable the ByView methods
2 parents 26f6919 + df816a8 commit 9767bbf

File tree

10 files changed

+285
-7
lines changed

10 files changed

+285
-7
lines changed

engine/execution/ingestion/throttle_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,3 +242,13 @@ func (h *headerStore) Store(proposal *flow.ProposalHeader) error {
242242
func (h *headerStore) ProposalByBlockID(blockID flow.Identifier) (*flow.ProposalHeader, error) {
243243
return nil, nil
244244
}
245+
246+
func (h *headerStore) ByView(view uint64) (*flow.Header, error) {
247+
// Find header with matching view
248+
for _, header := range h.byID {
249+
if header.View == view {
250+
return header, nil
251+
}
252+
}
253+
return nil, fmt.Errorf("no header found for view %d", view)
254+
}

storage/blocks.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,16 @@ type Blocks interface {
6363
// Certified blocks are the blocks that have received a QC. Hotstuff guarantees that for each view,
6464
// at most one block is certified. Hence, the return value of `ByView` is guaranteed to be unique
6565
// even for non-finalized blocks.
66+
//
6667
// Expected errors during normal operations:
6768
// - `storage.ErrNotFound` if no certified block is known at given view.
69+
ByView(view uint64) (*flow.Block, error)
70+
71+
// ProposalByView returns the block proposal with the given view. It is only available for certified blocks.
6872
//
69-
// TODO: this method is not available until next spork (mainnet27) or a migration that builds the index.
70-
// ByView(view uint64) (*flow.Header, error)
73+
// Expected errors during normal operations:
74+
// - `storage.ErrNotFound` if no certified block is known at given view.
75+
ProposalByView(view uint64) (*flow.Proposal, error)
7176

7277
// ByCollectionID returns the block for the given [flow.CollectionGuarantee] ID.
7378
// This method is only available for collections included in finalized blocks.

storage/headers.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,10 @@ type Headers interface {
2121
// Certified blocks are the blocks that have received QC. Hotstuff guarantees that for each view,
2222
// at most one block is certified. Hence, the return value of `ByView` is guaranteed to be unique
2323
// even for non-finalized blocks.
24+
//
2425
// Expected errors during normal operations:
2526
// - [storage.ErrNotFound] if no certified block is known at given view.
26-
//
27-
// TODO: this method is not available until next spork (mainnet27) or a migration that builds the index.
28-
// ByView(view uint64) (*flow.Header, error)
27+
ByView(view uint64) (*flow.Header, error)
2928

3029
// Exists returns true if a header with the given ID has been stored.
3130
// No errors are expected during normal operation.

storage/mock/blocks.go

Lines changed: 60 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

storage/mock/headers.go

Lines changed: 30 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

storage/operation/headers_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"testing"
55
"time"
66

7+
"github.com/jordanschalm/lockctx"
78
"github.com/stretchr/testify/assert"
89
"github.com/stretchr/testify/require"
910

@@ -90,3 +91,25 @@ func TestBlockHeightIndexLookup(t *testing.T) {
9091
assert.Equal(t, expected, actual)
9192
})
9293
}
94+
95+
func TestBlockViewIndexLookup(t *testing.T) {
96+
dbtest.RunWithDB(t, func(t *testing.T, db storage.DB) {
97+
98+
view := uint64(1337)
99+
expected := flow.Identifier{0x01, 0x02, 0x03}
100+
101+
lockManager := storage.NewTestingLockManager()
102+
103+
unittest.WithLock(t, lockManager, storage.LockInsertBlock, func(lctx lockctx.Context) error {
104+
return db.WithReaderBatchWriter(func(rw storage.ReaderBatchWriter) error {
105+
return operation.IndexCertifiedBlockByView(lctx, rw, view, expected)
106+
})
107+
})
108+
109+
var actual flow.Identifier
110+
err := operation.LookupCertifiedBlockByView(db.Reader(), view, &actual)
111+
require.NoError(t, err)
112+
113+
assert.Equal(t, expected, actual)
114+
})
115+
}

storage/store/blocks.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,18 @@ func (b *Blocks) ByView(view uint64) (*flow.Block, error) {
149149
return b.ByID(blockID)
150150
}
151151

152+
// ProposalByView returns the block proposal with the given view. It is only available for certified blocks.
153+
//
154+
// Expected errors during normal operations:
155+
// - `storage.ErrNotFound` if no certified block is known at given view.
156+
func (b *Blocks) ProposalByView(view uint64) (*flow.Proposal, error) {
157+
blockID, err := b.headers.BlockIDByView(view)
158+
if err != nil {
159+
return nil, err
160+
}
161+
return b.retrieveProposal(blockID)
162+
}
163+
152164
// ByHeight returns the block at the given height. It is only available
153165
// for finalized blocks.
154166
//

storage/store/blocks_test.go

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@ package store_test
33
import (
44
"testing"
55

6+
"github.com/jordanschalm/lockctx"
67
"github.com/stretchr/testify/require"
78

89
"github.com/onflow/flow-go/module/metrics"
910
"github.com/onflow/flow-go/storage"
11+
"github.com/onflow/flow-go/storage/operation"
1012
"github.com/onflow/flow-go/storage/operation/dbtest"
1113
"github.com/onflow/flow-go/storage/store"
1214
"github.com/onflow/flow-go/utils/unittest"
@@ -52,3 +54,107 @@ func TestBlockStoreAndRetrieve(t *testing.T) {
5254
require.Equal(t, *block, *receivedAfterRestart)
5355
})
5456
}
57+
58+
func TestBlockIndexByHeightAndRetrieve(t *testing.T) {
59+
dbtest.RunWithDB(t, func(t *testing.T, db storage.DB) {
60+
lockManager := storage.NewTestingLockManager()
61+
cacheMetrics := &metrics.NoopCollector{}
62+
blocks := store.InitAll(cacheMetrics, db).Blocks
63+
block := unittest.FullBlockFixture()
64+
prop := unittest.ProposalFromBlock(block)
65+
66+
// First store the block
67+
unittest.WithLock(t, lockManager, storage.LockInsertBlock, func(lctx lockctx.Context) error {
68+
return db.WithReaderBatchWriter(func(rw storage.ReaderBatchWriter) error {
69+
return blocks.BatchStore(lctx, rw, prop)
70+
})
71+
})
72+
73+
// Now index the block by height (requires LockFinalizeBlock)
74+
unittest.WithLock(t, lockManager, storage.LockFinalizeBlock, func(lctx lockctx.Context) error {
75+
return db.WithReaderBatchWriter(func(rw storage.ReaderBatchWriter) error {
76+
return operation.IndexFinalizedBlockByHeight(lctx, rw, block.Height, block.ID())
77+
})
78+
})
79+
80+
// Verify we can retrieve the block by height
81+
retrievedByHeight, err := blocks.ByHeight(block.Height)
82+
require.NoError(t, err)
83+
require.Equal(t, *block, *retrievedByHeight)
84+
85+
// Verify we can retrieve the proposal by height
86+
retrievedProposalByHeight, err := blocks.ProposalByHeight(block.Height)
87+
require.NoError(t, err)
88+
require.Equal(t, *prop, *retrievedProposalByHeight)
89+
90+
// Test that indexing the same height again returns ErrAlreadyExists
91+
unittest.WithLock(t, lockManager, storage.LockFinalizeBlock, func(lctx lockctx.Context) error {
92+
err := db.WithReaderBatchWriter(func(rw storage.ReaderBatchWriter) error {
93+
return operation.IndexFinalizedBlockByHeight(lctx, rw, block.Height, block.ID())
94+
})
95+
require.ErrorIs(t, err, storage.ErrAlreadyExists)
96+
return nil
97+
})
98+
99+
// Test that retrieving by non-existent height returns ErrNotFound
100+
_, err = blocks.ByHeight(block.Height + 1000)
101+
require.ErrorIs(t, err, storage.ErrNotFound)
102+
103+
// Verify after a restart, the block indexed by height is still retrievable
104+
blocksAfterRestart := store.InitAll(cacheMetrics, db).Blocks
105+
receivedAfterRestart, err := blocksAfterRestart.ByHeight(block.Height)
106+
require.NoError(t, err)
107+
require.Equal(t, *block, *receivedAfterRestart)
108+
})
109+
}
110+
111+
func TestBlockIndexByViewAndRetrieve(t *testing.T) {
112+
dbtest.RunWithDB(t, func(t *testing.T, db storage.DB) {
113+
lockManager := storage.NewTestingLockManager()
114+
cacheMetrics := &metrics.NoopCollector{}
115+
blocks := store.InitAll(cacheMetrics, db).Blocks
116+
block := unittest.FullBlockFixture()
117+
prop := unittest.ProposalFromBlock(block)
118+
119+
// First store the block and index by view
120+
unittest.WithLock(t, lockManager, storage.LockInsertBlock, func(lctx lockctx.Context) error {
121+
return db.WithReaderBatchWriter(func(rw storage.ReaderBatchWriter) error {
122+
err := blocks.BatchStore(lctx, rw, prop)
123+
if err != nil {
124+
return err
125+
}
126+
// Now index the block by view (requires LockInsertBlock)
127+
return operation.IndexCertifiedBlockByView(lctx, rw, block.View, block.ID())
128+
})
129+
})
130+
131+
// Verify we can retrieve the block by view
132+
retrievedByView, err := blocks.ByView(block.View)
133+
require.NoError(t, err)
134+
require.Equal(t, *block, *retrievedByView)
135+
136+
// Verify we can retrieve the proposal by view
137+
retrievedProposalByView, err := blocks.ProposalByView(block.View)
138+
require.NoError(t, err)
139+
require.Equal(t, *prop, *retrievedProposalByView)
140+
141+
// Test that indexing the same view again returns ErrAlreadyExists
142+
unittest.WithLock(t, lockManager, storage.LockInsertBlock, func(lctx lockctx.Context) error {
143+
err := db.WithReaderBatchWriter(func(rw storage.ReaderBatchWriter) error {
144+
return operation.IndexCertifiedBlockByView(lctx, rw, block.View, block.ID())
145+
})
146+
require.ErrorIs(t, err, storage.ErrAlreadyExists)
147+
return nil
148+
})
149+
150+
// Test that retrieving by non-existent view returns ErrNotFound
151+
_, err = blocks.ByView(block.View + 1000)
152+
require.ErrorIs(t, err, storage.ErrNotFound)
153+
154+
// Verify after a restart, the block indexed by view is still retrievable
155+
blocksAfterRestart := store.InitAll(cacheMetrics, db).Blocks
156+
receivedAfterRestart, err := blocksAfterRestart.ByView(block.View)
157+
require.NoError(t, err)
158+
require.Equal(t, *block, *receivedAfterRestart)
159+
})
160+
}

storage/store/headers.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,9 @@ func (h *Headers) ByHeight(height uint64) (*flow.Header, error) {
145145
// Certified blocks are the blocks that have received QC. Hotstuff guarantees that for each view,
146146
// at most one block is certified. Hence, the return value of `ByView` is guaranteed to be unique
147147
// even for non-finalized blocks.
148+
//
148149
// Expected errors during normal operations:
149150
// - [storage.ErrNotFound] if no certified block is known at given view.
150-
//
151-
// NOTE: this method is not available until next spork (mainnet27) or a migration that builds the index.
152151
func (h *Headers) ByView(view uint64) (*flow.Header, error) {
153152
blockID, err := h.viewCache.Get(h.db.Reader(), view)
154153
if err != nil {

storage/store/headers_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package store_test
33
import (
44
"testing"
55

6+
"github.com/jordanschalm/lockctx"
7+
68
"github.com/onflow/flow-go/storage/operation"
79
"github.com/onflow/flow-go/storage/operation/dbtest"
810
"github.com/onflow/flow-go/storage/store"
@@ -52,6 +54,38 @@ func TestHeaderStoreRetrieve(t *testing.T) {
5254
})
5355
}
5456

57+
func TestHeaderIndexByViewAndRetrieve(t *testing.T) {
58+
dbtest.RunWithDB(t, func(t *testing.T, db storage.DB) {
59+
lockManager := storage.NewTestingLockManager()
60+
metrics := metrics.NewNoopCollector()
61+
all := store.InitAll(metrics, db)
62+
headers := all.Headers
63+
blocks := all.Blocks
64+
65+
proposal := unittest.ProposalFixture()
66+
block := proposal.Block
67+
68+
// store block which will also store header
69+
unittest.WithLock(t, lockManager, storage.LockInsertBlock, func(lctx lockctx.Context) error {
70+
return db.WithReaderBatchWriter(func(rw storage.ReaderBatchWriter) error {
71+
return blocks.BatchStore(lctx, rw, proposal)
72+
})
73+
})
74+
75+
unittest.WithLock(t, lockManager, storage.LockInsertBlock, func(lctx lockctx.Context) error {
76+
return db.WithReaderBatchWriter(func(rw storage.ReaderBatchWriter) error {
77+
// index the header
78+
return operation.IndexCertifiedBlockByView(lctx, rw, block.View, block.ID())
79+
})
80+
})
81+
82+
// retrieve header by view
83+
actual, err := headers.ByView(block.View)
84+
require.NoError(t, err)
85+
require.Equal(t, block.ToHeader(), actual)
86+
})
87+
}
88+
5589
func TestHeaderRetrieveWithoutStore(t *testing.T) {
5690
dbtest.RunWithDB(t, func(t *testing.T, db storage.DB) {
5791
metrics := metrics.NewNoopCollector()

0 commit comments

Comments
 (0)