Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ go-test:
go test -v github.com/onflow/rosetta/state/...
go test -v github.com/onflow/rosetta/script/...

# End-to-end localnet compatibility test (script/README.md). Requires a flow-go
# localnet up at 127.0.0.1:4001, the flow CLI, jq, and python3 with click +
# requests; skips cleanly if any are absent. Build-tagged out of go-test.
.PHONY: localnet-test
localnet-test:
go test -tags localnet -v -timeout 20m github.com/onflow/rosetta/localnettest/...

.PHONY: gen-originator-account
gen-originator-account:
KEYS=$$(go run ./cmd/genkey/genkey.go -csv); \
Expand Down Expand Up @@ -68,9 +75,9 @@ create-originator-derived-account:
ROOT_ORIGINATOR_PRIVATE_KEY=$$(grep '$(ORIGINATOR_NAME)' $(ACCOUNT_KEYS_FILENAME) | cut -d ',' -f4 ); \
ROOT_ORIGINATOR_ADDRESS=$$(grep '$(ORIGINATOR_NAME)' $(ACCOUNT_KEYS_FILENAME) | cut -d ',' -f5); \
echo "Originator address: $$ROOT_ORIGINATOR_ADDRESS"; \
TX_HASH=$$(python3 rosetta_handler.py rosetta-create-derived-account $(ROSETTA_HOST_URL) $$ROOT_ORIGINATOR_ADDRESS $$ROOT_ORIGINATOR_PUBLIC_KEY $$ROOT_ORIGINATOR_PRIVATE_KEY $$NEW_ACCOUNT_PUBLIC_ROSETTA_KEY); \
ADDRESS=$$(flow transactions get $$TX_HASH -f $(FLOW_JSON) -n $(ROSETTA_ENV) -o json | jq -r '.events[] | select(.type == "flow.AccountCreated") | .values.value.fields[] | select(.name == "address") | .value.value'); \
echo "TX_HASH: $$TX_HASH , ADDRESS: $$ADDRESS"; \
TX_HASH=$$(python3 rosetta_handler.py rosetta-create-derived-account $(ROSETTA_HOST_URL) $$ROOT_ORIGINATOR_ADDRESS $$ROOT_ORIGINATOR_PUBLIC_KEY $$ROOT_ORIGINATOR_PRIVATE_KEY $$NEW_ACCOUNT_PUBLIC_ROSETTA_KEY) && \
ADDRESS=$$(flow transactions get $$TX_HASH -f $(FLOW_JSON) -n $(ROSETTA_ENV) -o json | jq -r '.events[] | select(.type == "flow.AccountCreated") | .values.value.fields[] | select(.name == "address") | .value.value') && \
echo "TX_HASH: $$TX_HASH , ADDRESS: $$ADDRESS" && \
echo "$(NEW_ACCOUNT_NAME),$$NEW_ACCOUNT_PUBLIC_FLOW_KEY,$$NEW_ACCOUNT_PUBLIC_ROSETTA_KEY,$$NEW_ACCOUNT_PRIVATE_KEY,$$ADDRESS" >> $(ACCOUNT_KEYS_FILENAME);

.PHONY: rosetta-transfer-funds
Expand Down
83 changes: 43 additions & 40 deletions indexdb/indexdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ var (
ErrBlockNotIndexed = errors.New("indexdb: block not indexed")
)

var (
accountPrefix = []byte("a")
blockPrefix = []byte("b")
hash2HeightPrefix = []byte("c")
height2HashPrefix = []byte("d")
isProxyPrefix = []byte("p")
)

Comment on lines +28 to +35

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Consider defining byte constants to fix inconsistencies and avoid potential slice-append races.

While centralizing the prefix definitions is a great refactor, defining them solely as global slices creates an unsafe pattern later in the file where append(prefix, ...) is called on them. If the capacity of a global slice happens to be larger than its length, concurrent append calls will write to the same backing array, causing a data race.

Additionally, because accountPrefix is a slice, other areas of the code (like line 315) are still forced to hardcode key[0] = 'a', defeating the purpose of centralizing the prefixes.

Consider defining the exact bytes as constants, and deriving the slices from them for iterators:

const (
	accountPrefixByte     = 'a'
	blockPrefixByte       = 'b'
	hash2HeightPrefixByte = 'c'
	height2HashPrefixByte = 'd'
	isProxyPrefixByte     = 'p'
)

var (
	accountPrefix     = []byte{accountPrefixByte}
	blockPrefix       = []byte{blockPrefixByte}
	hash2HeightPrefix = []byte{hash2HeightPrefixByte}
	height2HashPrefix = []byte{height2HashPrefixByte}
	isProxyPrefix     = []byte{isProxyPrefixByte}
)

This allows the rest of the file to use key[0] = accountPrefixByte safely and cleanly, while avoiding the unsafe append on global slices.


🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Consider defining byte constants to fix inconsistencies and avoid potential slice-append races.

While centralizing the prefix definitions is a great refactor, defining them solely as global slices creates an unsafe pattern later in the file where append(prefix, ...) is called on them. If the capacity of a global slice happens to be larger than its length, concurrent append calls will write to the same backing array, causing a data race.

Additionally, because accountPrefix is a slice, other areas of the code (like line 315) are still forced to hardcode key[0] = 'a', defeating the purpose of centralizing the prefixes.

Consider defining the exact bytes as constants, and deriving the slices from them for iterators:

const (
	accountPrefixByte     = 'a'
	blockPrefixByte       = 'b'
	hash2HeightPrefixByte = 'c'
	height2HashPrefixByte = 'd'
	isProxyPrefixByte     = 'p'
)

var (
	accountPrefix     = []byte{accountPrefixByte}
	blockPrefix       = []byte{blockPrefixByte}
	hash2HeightPrefix = []byte{hash2HeightPrefixByte}
	height2HashPrefix = []byte{height2HashPrefixByte}
	isProxyPrefix     = []byte{isProxyPrefixByte}
)

This allows the rest of the file to use key[0] = accountPrefixByte safely and cleanly, while avoiding the unsafe append on global slices.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indexdb/indexdb.go` around lines 28 - 35, Define byte constants for each
prefix alongside the existing prefix slices, then initialize accountPrefix,
blockPrefix, hash2HeightPrefix, height2HashPrefix, and isProxyPrefix from those
constants. Update hardcoded prefix assignments such as key[0] in the relevant
code to use the corresponding constant, while retaining the slices for iterator
usage and avoiding direct append operations on shared global slices.

// NOTE(tav): We store the blockchain data within Badger using the following
// key/value structure:
//
Expand Down Expand Up @@ -80,10 +88,9 @@ func (s *Store) Accounts() (map[[8]byte]bool, error) {
err := s.db.View(func(txn *badger.Txn) error {
it := txn.NewIterator(badger.IteratorOptions{})
defer it.Close()
prefix := []byte("a")
it.Seek(prefix)
it.Seek(accountPrefix)
for {
if !it.ValidForPrefix(prefix) {
if !it.ValidForPrefix(accountPrefix) {
break
}
key := it.Item().Key()
Expand All @@ -94,10 +101,9 @@ func (s *Store) Accounts() (map[[8]byte]bool, error) {
}
it = txn.NewIterator(badger.IteratorOptions{})
defer it.Close()
prefix = []byte("p")
it.Seek(prefix)
it.Seek(isProxyPrefix)
for {
if !it.ValidForPrefix(prefix) {
if !it.ValidForPrefix(isProxyPrefix) {
break
}
key := it.Item().Key()
Expand Down Expand Up @@ -127,10 +133,9 @@ func (s *Store) AccountsInfo() (map[string]*AccountInfo, error) {
err := s.db.View(func(txn *badger.Txn) error {
it := txn.NewIterator(badger.IteratorOptions{})
defer it.Close()
prefix := []byte("a")
it.Seek(prefix)
it.Seek(accountPrefix)
for {
if !it.ValidForPrefix(prefix) {
if !it.ValidForPrefix(accountPrefix) {
break
}
item := it.Item()
Expand Down Expand Up @@ -163,10 +168,9 @@ func (s *Store) AccountsInfo() (map[string]*AccountInfo, error) {
err = s.db.View(func(txn *badger.Txn) error {
it := txn.NewIterator(badger.IteratorOptions{})
defer it.Close()
prefix := []byte("p")
it.Seek(prefix)
it.Seek(isProxyPrefix)
for {
if !it.ValidForPrefix(prefix) {
if !it.ValidForPrefix(isProxyPrefix) {
break
}
key := it.Item().Key()
Expand Down Expand Up @@ -308,7 +312,7 @@ func (s *Store) Genesis() *model.BlockMeta {
// given height.
func (s *Store) HasBalance(acct []byte, height uint64) (bool, error) {
key := make([]byte, 1+8+8)
key[0] = 'a'
key[0] = 'a' // accountPrefix
copy(key[1:9], acct)
binary.BigEndian.PutUint64(key[9:], height)
ok := false
Expand Down Expand Up @@ -337,7 +341,7 @@ func (s *Store) HashForHeight(height uint64) ([]byte, error) {
var hash []byte
heightEnc := make([]byte, 8)
binary.BigEndian.PutUint64(heightEnc, height)
key := append([]byte("d"), heightEnc...)
key := append(height2HashPrefix, heightEnc...)
err := s.db.View(func(txn *badger.Txn) error {
Comment on lines 341 to 345

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Data race hazard: append on global slices.

Using append on a global slice variable is unsafe in concurrent code. If the global slice has any spare capacity (cap > len), concurrent append calls will write over each other in the same backing array, leading to a data race and index corruption. Pre-allocate the target slice and use copy to guarantee thread safety.

  • indexdb/indexdb.go#L341-L345: Replace append(height2HashPrefix, heightEnc...) with a safely pre-allocated slice key := make([]byte, 1+8); key[0] = height2HashPrefix[0]; copy(key[1:], heightEnc).
  • indexdb/indexdb.go#L368-L372: Replace append(hash2HeightPrefix, hash...) with safe pre-allocation and copy().
  • indexdb/indexdb.go#L456-L462: Replace append on blockPrefix, hash2HeightPrefix, and height2HashPrefix with safe pre-allocation and copy().
  • indexdb/indexdb.go#L781-L789: Replace append on blockPrefix, hash2HeightPrefix, and height2HashPrefix with safe pre-allocation and copy().
📍 Affects 1 file
  • indexdb/indexdb.go#L341-L345 (this comment)
  • indexdb/indexdb.go#L368-L372
  • indexdb/indexdb.go#L456-L462
  • indexdb/indexdb.go#L781-L789
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indexdb/indexdb.go` around lines 341 - 345, Eliminate concurrent append
operations on shared prefix slices in indexdb/indexdb.go at lines 341-345,
368-372, 456-462, and 781-789. In the affected indexdb methods, allocate each
key buffer with its exact required length, copy the prefix and payload into it,
and preserve the existing key contents and ordering; update all listed
blockPrefix, hash2HeightPrefix, and height2HashPrefix constructions, with no
direct changes needed beyond these replacements.

item, err := txn.Get(key)
if err != nil {
Expand All @@ -363,7 +367,7 @@ func (s *Store) HashForHeight(height uint64) ([]byte, error) {
// HeightForHash returns the block height for the given hash.
func (s *Store) HeightForHash(hash []byte) (uint64, error) {
height := uint64(0)
key := append([]byte("c"), hash...)
key := append(hash2HeightPrefix, hash...)
err := s.db.View(func(txn *badger.Txn) error {
item, err := txn.Get(key)
if err != nil {
Expand Down Expand Up @@ -423,7 +427,7 @@ func (s *Store) Index(ctx context.Context, height uint64, hash []byte, block *mo
}
if len(op.ProxyPublicKey) > 0 {
key := make([]byte, 17)
key[0] = 'p'
key[0] = 'p' // isProxyPrefix
copy(key[1:], op.Account)
binary.BigEndian.PutUint64(key[9:], height)
proxyAccts = append(proxyAccts, key)
Expand All @@ -440,7 +444,7 @@ func (s *Store) Index(ctx context.Context, height uint64, hash []byte, block *mo
updates := make([]accountUpdate, len(accts))
for acct, diff := range accts {
key := make([]byte, 1+8+8)
key[0] = 'a'
key[0] = 'a' // accountPrefix
copy(key[1:], acct)
copy(key[9:], hval)
updates[i] = accountUpdate{
Expand All @@ -449,13 +453,13 @@ func (s *Store) Index(ctx context.Context, height uint64, hash []byte, block *mo
}
i++
}
blockKey := append([]byte("b"), hval...)
blockKey := append(blockPrefix, hval...)
blockValue, err := proto.Marshal(block)
if err != nil {
log.Fatalf("Failed to encode model.IndexedBlock: %s", err)
}
hash2heightKey := append([]byte("c"), hash...)
height2hashKey := append([]byte("d"), hval...)
hash2heightKey := append(hash2HeightPrefix, hash...)
height2hashKey := append(height2HashPrefix, hval...)
latest = &model.BlockMeta{
Hash: hash,
Height: height,
Expand Down Expand Up @@ -576,10 +580,9 @@ func (s *Store) PurgeProxyAccounts() {
err := s.db.View(func(txn *badger.Txn) error {
it := txn.NewIterator(badger.IteratorOptions{})
defer it.Close()
prefix := []byte("p")
it.Seek(prefix)
it.Seek(isProxyPrefix)
for {
if !it.ValidForPrefix(prefix) {
if !it.ValidForPrefix(isProxyPrefix) {
break
}
key := it.Item().KeyCopy(nil)
Expand All @@ -595,7 +598,7 @@ func (s *Store) PurgeProxyAccounts() {
err = s.db.View(func(txn *badger.Txn) error {
it := txn.NewIterator(badger.IteratorOptions{})
defer it.Close()
prefix := []byte("a")
prefix := accountPrefix
it.Seek(prefix)
for {
if !it.ValidForPrefix(prefix) {
Expand Down Expand Up @@ -640,7 +643,7 @@ func (s *Store) ResetTo(base uint64) error {
delKeys := [][]byte{}
err := s.db.View(func(txn *badger.Txn) error {
it := txn.NewIterator(badger.IteratorOptions{})
prefix := []byte("a")
prefix := accountPrefix
it.Seek(prefix)
for {
if !it.ValidForPrefix(prefix) {
Expand All @@ -662,7 +665,7 @@ func (s *Store) ResetTo(base uint64) error {
}
err = s.db.View(func(txn *badger.Txn) error {
it := txn.NewIterator(badger.IteratorOptions{})
prefix := []byte("p")
prefix := isProxyPrefix
it.Seek(prefix)
for {
if !it.ValidForPrefix(prefix) {
Expand All @@ -685,7 +688,7 @@ func (s *Store) ResetTo(base uint64) error {
last := uint64(0)
err = s.db.View(func(txn *badger.Txn) error {
it := txn.NewIterator(badger.IteratorOptions{})
prefix := []byte("d")
prefix := height2HashPrefix
it.Seek(prefix)
for {
if !it.ValidForPrefix(prefix) {
Expand All @@ -695,21 +698,21 @@ func (s *Store) ResetTo(base uint64) error {
key := item.Key()
height := binary.BigEndian.Uint64(key[1:])
if height > base {
key = item.KeyCopy(nil)
delKeys = append(delKeys, key)
key = make([]byte, 9)
key[0] = 'b'
binary.BigEndian.PutUint64(key[1:], height)
delKeys = append(delKeys, key)
height2HashKey := item.KeyCopy(nil)
delKeys = append(delKeys, height2HashKey)
blockKey := make([]byte, 9)
blockKey[0] = 'b' // blockPrefix
binary.BigEndian.PutUint64(blockKey[1:], height)
delKeys = append(delKeys, blockKey)
hash, err := item.ValueCopy(nil)
if err != nil {
it.Close()
return err
}
key = make([]byte, len(hash)+1)
key[0] = 'c'
copy(key[1:], hash)
delKeys = append(delKeys, key)
hash2HeightKey := make([]byte, len(hash)+1)
hash2HeightKey[0] = 'c' // hash2HeightPrefix
copy(hash2HeightKey[1:], hash)
delKeys = append(delKeys, hash2HeightKey)
} else {
last = height
}
Expand Down Expand Up @@ -775,15 +778,15 @@ func (s *Store) SetGenesis(val *model.BlockMeta) error {
}
hval := make([]byte, 8)
binary.BigEndian.PutUint64(hval, val.Height)
blockKey := append([]byte("b"), hval...)
blockKey := append(blockPrefix, hval...)
blockValue, err := proto.Marshal(&model.IndexedBlock{
Timestamp: val.Timestamp,
})
if err != nil {
log.Fatalf("Failed to encode model.IndexedBlock: %s", err)
}
hash2heightKey := append([]byte("c"), val.Hash...)
height2hashKey := append([]byte("d"), hval...)
hash2heightKey := append(hash2HeightPrefix, val.Hash...)
height2hashKey := append(height2HashPrefix, hval...)
err = s.db.Update(func(txn *badger.Txn) error {
if err := txn.Set([]byte("genesis"), genesis); err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion localnet.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
}
],
"root_block": 0,
"version": 7
"version": 8
}
}
}
17 changes: 17 additions & 0 deletions localnettest/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Package localnettest holds an end-to-end localnet compatibility test for the
// Rosetta server, automating the validation procedure in script/README.md.
//
// The test is guarded by the `localnet` build tag and is excluded from
// `make go-test`; run it with `make localnet-test`. It requires a flow-go
// localnet running at 127.0.0.1:4001 (see flow-go integration/localnet, built
// at the flow-go version under test), plus the flow CLI, jq, and python3 with
// `click` and `requests`. It skips cleanly when any of those are unavailable.
//
// It bootstraps and funds an originator, starts ./server against localnet,
// waits for the indexer to reach tip, then creates a derived account and
// transfers funds through Rosetta's construction API — asserting via the
// Rosetta REST API that the server indexes the network without a block-ID
// mismatch and observes the transfer. A failure here means the current Rosetta
// build is incompatible with the localnet's flow-go version (or its spork
// config is stale), which is exactly the signal the upgrade procedure needs.
package localnettest
Loading
Loading