From 6c8594439a6961bfaa2f40431a74ec533dc8604f Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Wed, 13 Dec 2023 10:26:36 -0800 Subject: [PATCH 1/4] [Ledger] Refactor NewRegisterID to accept address --- .../migrations/storage_fees_migration.go | 2 +- cmd/util/ledger/migrations/utils.go | 4 +- cmd/util/ledger/reporters/atree_reporter.go | 2 +- cmd/util/ledger/reporters/storage_snapshot.go | 2 +- engine/execution/scripts/engine.go | 2 +- fvm/environment/accounts.go | 2 +- fvm/environment/accounts_test.go | 22 ++++----- .../derived_data_invalidator_test.go | 10 ++-- fvm/meter/meter_test.go | 34 +++++++------- fvm/storage/derived/table_test.go | 3 +- fvm/storage/snapshot/snapshot_tree_test.go | 8 ++-- fvm/storage/state/execution_state_test.go | 29 ++++++------ fvm/storage/state/spock_state_test.go | 46 +++++++++++-------- fvm/storage/state/storage_state_test.go | 28 ++++++----- fvm/storage/state/transaction_state_test.go | 28 ++++++----- fvm/transactionStorageLimiter_test.go | 4 +- ledger/common/convert/convert.go | 2 +- ledger/common/convert/convert_test.go | 14 +++--- ledger/partial/ledger_test.go | 2 +- model/flow/ledger.go | 8 ++-- model/flow/ledger_test.go | 38 +++++++-------- module/chunks/chunkVerifier_test.go | 13 ++---- .../indexer/indexer_core_test.go | 2 +- storage/badger/operation/interactions_test.go | 12 ++--- storage/pebble/registers_test.go | 5 +- utils/debug/registerCache.go | 6 +-- utils/unittest/fixtures.go | 11 +++-- 27 files changed, 182 insertions(+), 157 deletions(-) diff --git a/cmd/util/ledger/migrations/storage_fees_migration.go b/cmd/util/ledger/migrations/storage_fees_migration.go index 6babd9ed279..b7dd49a1ec6 100644 --- a/cmd/util/ledger/migrations/storage_fees_migration.go +++ b/cmd/util/ledger/migrations/storage_fees_migration.go @@ -25,7 +25,7 @@ func StorageFeesMigration(payload []ledger.Payload) ([]ledger.Payload, error) { for s, u := range storageUsed { // this is the storage used by the storage_used register we are about to add id := flow.NewRegisterID( - string(flow.BytesToAddress([]byte(s)).Bytes()), + flow.BytesToAddress([]byte(s)), "storage_used") storageUsedByStorageUsed := fvm.RegisterSize(id, make([]byte, 8)) u = u + uint64(storageUsedByStorageUsed) diff --git a/cmd/util/ledger/migrations/utils.go b/cmd/util/ledger/migrations/utils.go index 11510008f74..e747b3dc508 100644 --- a/cmd/util/ledger/migrations/utils.go +++ b/cmd/util/ledger/migrations/utils.go @@ -22,7 +22,7 @@ var _ atree.Ledger = &AccountsAtreeLedger{} func (a *AccountsAtreeLedger) GetValue(owner, key []byte) ([]byte, error) { v, err := a.Accounts.GetValue( flow.NewRegisterID( - string(flow.BytesToAddress(owner).Bytes()), + flow.BytesToAddress(owner), string(key))) if err != nil { return nil, fmt.Errorf("getting value failed: %w", err) @@ -33,7 +33,7 @@ func (a *AccountsAtreeLedger) GetValue(owner, key []byte) ([]byte, error) { func (a *AccountsAtreeLedger) SetValue(owner, key, value []byte) error { err := a.Accounts.SetValue( flow.NewRegisterID( - string(flow.BytesToAddress(owner).Bytes()), + flow.BytesToAddress(owner), string(key)), value) if err != nil { diff --git a/cmd/util/ledger/reporters/atree_reporter.go b/cmd/util/ledger/reporters/atree_reporter.go index 6d1be625125..39c005dbc55 100644 --- a/cmd/util/ledger/reporters/atree_reporter.go +++ b/cmd/util/ledger/reporters/atree_reporter.go @@ -117,7 +117,7 @@ func getPayloadType(p *ledger.Payload) (payloadType, error) { } id := flow.NewRegisterID( - string(k.KeyParts[0].Value), + flow.BytesToAddress(k.KeyParts[0].Value), string(k.KeyParts[1].Value)) if id.IsInternalState() { return fvmPayloadType, nil diff --git a/cmd/util/ledger/reporters/storage_snapshot.go b/cmd/util/ledger/reporters/storage_snapshot.go index b9ca42c1fe5..bf4698e0559 100644 --- a/cmd/util/ledger/reporters/storage_snapshot.go +++ b/cmd/util/ledger/reporters/storage_snapshot.go @@ -19,7 +19,7 @@ func NewStorageSnapshotFromPayload( } id := flow.NewRegisterID( - string(key.KeyParts[0].Value), + flow.BytesToAddress(key.KeyParts[0].Value), string(key.KeyParts[1].Value)) snapshot[id] = entry.Value() diff --git a/engine/execution/scripts/engine.go b/engine/execution/scripts/engine.go index 689d7858223..ea46f273d73 100644 --- a/engine/execution/scripts/engine.go +++ b/engine/execution/scripts/engine.go @@ -74,7 +74,7 @@ func (e *Engine) GetRegisterAtBlockID( return nil, fmt.Errorf("failed to create storage snapshot: %w", err) } - id := flow.NewRegisterID(string(owner), string(key)) + id := flow.NewRegisterID(flow.BytesToAddress(owner), string(key)) data, err := blockSnapshot.Get(id) if err != nil { return nil, fmt.Errorf("failed to get the register (%s): %w", id, err) diff --git a/fvm/environment/accounts.go b/fvm/environment/accounts.go index 6af7c24aed5..b6ca1bd2d0a 100644 --- a/fvm/environment/accounts.go +++ b/fvm/environment/accounts.go @@ -75,7 +75,7 @@ func (a *StatefulAccounts) AllocateStorageIndex( key := atree.SlabIndexToLedgerKey(index) a.txnState.RunWithAllLimitsDisabled(func() { err = a.txnState.Set( - flow.NewRegisterID(string(address.Bytes()), string(key)), + flow.NewRegisterID(address, string(key)), []byte{}) }) if err != nil { diff --git a/fvm/environment/accounts_test.go b/fvm/environment/accounts_test.go index 7c4302278f2..c6ef3cce467 100644 --- a/fvm/environment/accounts_test.go +++ b/fvm/environment/accounts_test.go @@ -63,7 +63,7 @@ func TestAccounts_GetPublicKey(t *testing.T) { address := flow.HexToAddress("01") registerId := flow.NewRegisterID( - string(address.Bytes()), + address, "public_key_0") for _, value := range [][]byte{{}, nil} { @@ -88,7 +88,7 @@ func TestAccounts_GetPublicKeyCount(t *testing.T) { address := flow.HexToAddress("01") registerId := flow.NewRegisterID( - string(address.Bytes()), + address, "public_key_count") for _, value := range [][]byte{{}, nil} { @@ -114,7 +114,7 @@ func TestAccounts_GetPublicKeys(t *testing.T) { address := flow.HexToAddress("01") registerId := flow.NewRegisterID( - string(address.Bytes()), + address, "public_key_count") for _, value := range [][]byte{{}, nil} { @@ -243,7 +243,7 @@ func TestAccount_StorageUsed(t *testing.T) { txnState := testutils.NewSimpleTransaction(nil) accounts := environment.NewAccounts(txnState) address := flow.HexToAddress("01") - key := flow.NewRegisterID(string(address.Bytes()), "some_key") + key := flow.NewRegisterID(address, "some_key") err := accounts.Create(nil, address) require.NoError(t, err) @@ -260,7 +260,7 @@ func TestAccount_StorageUsed(t *testing.T) { txnState := testutils.NewSimpleTransaction(nil) accounts := environment.NewAccounts(txnState) address := flow.HexToAddress("01") - key := flow.NewRegisterID(string(address.Bytes()), "some_key") + key := flow.NewRegisterID(address, "some_key") err := accounts.Create(nil, address) require.NoError(t, err) @@ -279,7 +279,7 @@ func TestAccount_StorageUsed(t *testing.T) { txnState := testutils.NewSimpleTransaction(nil) accounts := environment.NewAccounts(txnState) address := flow.HexToAddress("01") - key := flow.NewRegisterID(string(address.Bytes()), "some_key") + key := flow.NewRegisterID(address, "some_key") err := accounts.Create(nil, address) require.NoError(t, err) @@ -298,7 +298,7 @@ func TestAccount_StorageUsed(t *testing.T) { txnState := testutils.NewSimpleTransaction(nil) accounts := environment.NewAccounts(txnState) address := flow.HexToAddress("01") - key := flow.NewRegisterID(string(address.Bytes()), "some_key") + key := flow.NewRegisterID(address, "some_key") err := accounts.Create(nil, address) require.NoError(t, err) @@ -317,7 +317,7 @@ func TestAccount_StorageUsed(t *testing.T) { txnState := testutils.NewSimpleTransaction(nil) accounts := environment.NewAccounts(txnState) address := flow.HexToAddress("01") - key := flow.NewRegisterID(string(address.Bytes()), "some_key") + key := flow.NewRegisterID(address, "some_key") err := accounts.Create(nil, address) require.NoError(t, err) @@ -340,19 +340,19 @@ func TestAccount_StorageUsed(t *testing.T) { err := accounts.Create(nil, address) require.NoError(t, err) - key1 := flow.NewRegisterID(string(address.Bytes()), "some_key") + key1 := flow.NewRegisterID(address, "some_key") err = accounts.SetValue(key1, createByteArray(12)) require.NoError(t, err) err = accounts.SetValue(key1, createByteArray(11)) require.NoError(t, err) - key2 := flow.NewRegisterID(string(address.Bytes()), "some_key2") + key2 := flow.NewRegisterID(address, "some_key2") err = accounts.SetValue(key2, createByteArray(22)) require.NoError(t, err) err = accounts.SetValue(key2, createByteArray(23)) require.NoError(t, err) - key3 := flow.NewRegisterID(string(address.Bytes()), "some_key3") + key3 := flow.NewRegisterID(address, "some_key3") err = accounts.SetValue(key3, createByteArray(22)) require.NoError(t, err) err = accounts.SetValue(key3, createByteArray(0)) diff --git a/fvm/environment/derived_data_invalidator_test.go b/fvm/environment/derived_data_invalidator_test.go index aa86aaeb258..026f0dd119a 100644 --- a/fvm/environment/derived_data_invalidator_test.go +++ b/fvm/environment/derived_data_invalidator_test.go @@ -300,19 +300,21 @@ func TestMeterParamOverridesUpdated(t *testing.T) { executionSnapshot, err = txnState.FinalizeMainTransaction() require.NoError(t, err) + owner := ctx.Chain.ServiceAddress() + otherOwner := unittest.RandomAddressFixtureForChain(ctx.Chain.ChainID()) + for _, registerId := range executionSnapshot.AllRegisterIDs() { checkForUpdates(registerId, true) checkForUpdates( - flow.NewRegisterID("other owner", registerId.Key), + flow.NewRegisterID(otherOwner, registerId.Key), false) } - owner := string(ctx.Chain.ServiceAddress().Bytes()) stabIndexKey := flow.NewRegisterID(owner, "$12345678") require.True(t, stabIndexKey.IsSlabIndex()) checkForUpdates(stabIndexKey, true) checkForUpdates(flow.NewRegisterID(owner, "other keys"), false) - checkForUpdates(flow.NewRegisterID("other owner", stabIndexKey.Key), false) - checkForUpdates(flow.NewRegisterID("other owner", "other key"), false) + checkForUpdates(flow.NewRegisterID(otherOwner, stabIndexKey.Key), false) + checkForUpdates(flow.NewRegisterID(otherOwner, "other key"), false) } diff --git a/fvm/meter/meter_test.go b/fvm/meter/meter_test.go index 46d9bbc32a5..4234966c260 100644 --- a/fvm/meter/meter_test.go +++ b/fvm/meter/meter_test.go @@ -408,7 +408,7 @@ func TestStorageLimits(t *testing.T) { meter.DefaultParameters(), ) - key1 := flow.NewRegisterID("", "1") + key1 := flow.NewRegisterID(flow.EmptyAddress, "1") val1 := []byte{0x1, 0x2, 0x3} size1 := meter.GetStorageKeyValueSizeForTesting(key1, val1) @@ -423,7 +423,7 @@ func TestStorageLimits(t *testing.T) { require.Equal(t, meter1.TotalBytesReadFromStorage(), size1) // first read of key2 - key2 := flow.NewRegisterID("", "2") + key2 := flow.NewRegisterID(flow.EmptyAddress, "2") val2 := []byte{0x3, 0x2, 0x1} size2 := meter.GetStorageKeyValueSizeForTesting(key2, val2) @@ -437,7 +437,7 @@ func TestStorageLimits(t *testing.T) { meter.DefaultParameters(), ) - key1 := flow.NewRegisterID("", "1") + key1 := flow.NewRegisterID(flow.EmptyAddress, "1") val1 := []byte{0x1, 0x2, 0x3} val2 := []byte{0x1, 0x2, 0x3, 0x4} @@ -452,7 +452,7 @@ func TestStorageLimits(t *testing.T) { require.Equal(t, meter1.TotalBytesWrittenToStorage(), meter.GetStorageKeyValueSizeForTesting(key1, val2)) // first write of key2 - key2 := flow.NewRegisterID("", "2") + key2 := flow.NewRegisterID(flow.EmptyAddress, "2") err = meter1.MeterStorageWrite(key2, val2, false) require.NoError(t, err) require.Equal(t, meter1.TotalBytesWrittenToStorage(), @@ -464,7 +464,7 @@ func TestStorageLimits(t *testing.T) { meter.DefaultParameters().WithStorageInteractionLimit(1), ) - key1 := flow.NewRegisterID("", "1") + key1 := flow.NewRegisterID(flow.EmptyAddress, "1") val1 := []byte{0x1, 0x2, 0x3} err := meter1.MeterStorageRead(key1, val1, false /* not enforced */) @@ -478,7 +478,7 @@ func TestStorageLimits(t *testing.T) { meter.DefaultParameters().WithStorageInteractionLimit(testLimit), ) - key1 := flow.NewRegisterID("", "1") + key1 := flow.NewRegisterID(flow.EmptyAddress, "1") val1 := []byte{0x1, 0x2, 0x3} err := meter1.MeterStorageRead(key1, val1, true /* enforced */) @@ -496,7 +496,7 @@ func TestStorageLimits(t *testing.T) { meter.DefaultParameters().WithStorageInteractionLimit(testLimit), ) - key1 := flow.NewRegisterID("", "1") + key1 := flow.NewRegisterID(flow.EmptyAddress, "1") val1 := []byte{0x1, 0x2, 0x3} err := meter1.MeterStorageWrite(key1, val1, false /* not enforced */) @@ -509,7 +509,7 @@ func TestStorageLimits(t *testing.T) { meter.DefaultParameters().WithStorageInteractionLimit(testLimit), ) - key1 := flow.NewRegisterID("", "1") + key1 := flow.NewRegisterID(flow.EmptyAddress, "1") val1 := []byte{0x1, 0x2, 0x3} err := meter1.MeterStorageWrite(key1, val1, true /* enforced */) @@ -526,8 +526,8 @@ func TestStorageLimits(t *testing.T) { meter.DefaultParameters(), ) - key1 := flow.NewRegisterID("", "1") - key2 := flow.NewRegisterID("", "2") + key1 := flow.NewRegisterID(flow.EmptyAddress, "1") + key2 := flow.NewRegisterID(flow.EmptyAddress, "2") val1 := []byte{0x1, 0x2, 0x3} val2 := []byte{0x1, 0x2, 0x3, 0x4} size1 := meter.GetStorageKeyValueSizeForTesting(key1, val1) @@ -547,8 +547,8 @@ func TestStorageLimits(t *testing.T) { }) t.Run("metering storage read and written - exceeding limit - not enforced", func(t *testing.T) { - key1 := flow.NewRegisterID("", "1") - key2 := flow.NewRegisterID("", "2") + key1 := flow.NewRegisterID(flow.EmptyAddress, "1") + key2 := flow.NewRegisterID(flow.EmptyAddress, "2") val1 := []byte{0x1, 0x2, 0x3} val2 := []byte{0x1, 0x2, 0x3, 0x4} size1 := meter.GetStorageKeyValueSizeForTesting(key1, val1) @@ -572,8 +572,8 @@ func TestStorageLimits(t *testing.T) { }) t.Run("metering storage read and written - exceeding limit - enforced", func(t *testing.T) { - key1 := flow.NewRegisterID("", "1") - key2 := flow.NewRegisterID("", "2") + key1 := flow.NewRegisterID(flow.EmptyAddress, "1") + key2 := flow.NewRegisterID(flow.EmptyAddress, "2") val1 := []byte{0x1, 0x2, 0x3} val2 := []byte{0x1, 0x2, 0x3, 0x4} size1 := meter.GetStorageKeyValueSizeForTesting(key1, val1) @@ -603,13 +603,13 @@ func TestStorageLimits(t *testing.T) { meter1 := meter.NewMeter( meter.DefaultParameters(), ) - readKey1 := flow.NewRegisterID("", "r1") + readKey1 := flow.NewRegisterID(flow.EmptyAddress, "r1") readVal1 := []byte{0x1, 0x2, 0x3} readSize1 := meter.GetStorageKeyValueSizeForTesting(readKey1, readVal1) err := meter1.MeterStorageRead(readKey1, readVal1, false) require.NoError(t, err) - writeKey1 := flow.NewRegisterID("", "w1") + writeKey1 := flow.NewRegisterID(flow.EmptyAddress, "w1") writeVal1 := []byte{0x1, 0x2, 0x3, 0x4} writeSize1 := meter.GetStorageKeyValueSizeForTesting(writeKey1, writeVal1) err = meter1.MeterStorageWrite(writeKey1, writeVal1, false) @@ -620,7 +620,7 @@ func TestStorageLimits(t *testing.T) { meter.DefaultParameters(), ) - writeKey2 := flow.NewRegisterID("", "w2") + writeKey2 := flow.NewRegisterID(flow.EmptyAddress, "w2") writeVal2 := []byte{0x1, 0x2, 0x3, 0x4, 0x5} writeSize2 := meter.GetStorageKeyValueSizeForTesting(writeKey2, writeVal2) diff --git a/fvm/storage/derived/table_test.go b/fvm/storage/derived/table_test.go index 774e9ba3003..2d9656c47de 100644 --- a/fvm/storage/derived/table_test.go +++ b/fvm/storage/derived/table_test.go @@ -12,6 +12,7 @@ import ( "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/fvm/storage/state" "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" ) func newEmptyTestBlock() *DerivedDataTable[string, *string] { @@ -962,7 +963,7 @@ func (computer *testValueComputer) Compute( func TestDerivedDataTableGetOrCompute(t *testing.T) { blockDerivedData := NewEmptyTable[flow.RegisterID, int](0) - key := flow.NewRegisterID("addr", "key") + key := flow.NewRegisterID(unittest.RandomAddressFixture(), "key") value := 12345 t.Run("compute value", func(t *testing.T) { diff --git a/fvm/storage/snapshot/snapshot_tree_test.go b/fvm/storage/snapshot/snapshot_tree_test.go index 5ccf83481e6..0395e861a7f 100644 --- a/fvm/storage/snapshot/snapshot_tree_test.go +++ b/fvm/storage/snapshot/snapshot_tree_test.go @@ -10,10 +10,10 @@ import ( ) func TestSnapshotTree(t *testing.T) { - id1 := flow.NewRegisterID("1", "") - id2 := flow.NewRegisterID("2", "") - id3 := flow.NewRegisterID("3", "") - missingId := flow.NewRegisterID("missing", "") + id1 := flow.NewRegisterID(flow.HexToAddress("0x1"), "") + id2 := flow.NewRegisterID(flow.HexToAddress("0x2"), "") + id3 := flow.NewRegisterID(flow.HexToAddress("0x3"), "") + missingId := flow.NewRegisterID(flow.HexToAddress("0x99"), "") value1v0 := flow.RegisterValue("1v0") diff --git a/fvm/storage/state/execution_state_test.go b/fvm/storage/state/execution_state_test.go index d12a1f34b6c..29961fa7e98 100644 --- a/fvm/storage/state/execution_state_test.go +++ b/fvm/storage/state/execution_state_test.go @@ -8,6 +8,7 @@ import ( "github.com/onflow/flow-go/fvm/meter" "github.com/onflow/flow-go/fvm/storage/state" "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" ) func createByteArray(size int) []byte { @@ -23,12 +24,12 @@ func TestExecutionState_Finalize(t *testing.T) { child := parent.NewChild() - readId := flow.NewRegisterID("0", "x") + readId := flow.NewRegisterID(unittest.RandomAddressFixture(), "x") _, err := child.Get(readId) require.NoError(t, err) - writeId := flow.NewRegisterID("1", "y") + writeId := flow.NewRegisterID(unittest.RandomAddressFixture(), "y") writeValue := flow.RegisterValue("a") err = child.Set(writeId, writeValue) @@ -65,8 +66,10 @@ func TestExecutionState_Finalize(t *testing.T) { func TestExecutionState_ChildMergeFunctionality(t *testing.T) { st := state.NewExecutionState(nil, state.DefaultParameters()) + owner := unittest.RandomAddressFixture() + t.Run("test read from parent state (backoff)", func(t *testing.T) { - key := flow.NewRegisterID("address", "key1") + key := flow.NewRegisterID(owner, "key1") value := createByteArray(1) // set key1 on parent err := st.Set(key, value) @@ -80,7 +83,7 @@ func TestExecutionState_ChildMergeFunctionality(t *testing.T) { }) t.Run("test write to child (no merge)", func(t *testing.T) { - key := flow.NewRegisterID("address", "key2") + key := flow.NewRegisterID(owner, "key2") value := createByteArray(2) stChild := st.NewChild() @@ -95,7 +98,7 @@ func TestExecutionState_ChildMergeFunctionality(t *testing.T) { }) t.Run("test write to child and merge", func(t *testing.T) { - key := flow.NewRegisterID("address", "key3") + key := flow.NewRegisterID(owner, "key3") value := createByteArray(3) stChild := st.NewChild() @@ -119,7 +122,7 @@ func TestExecutionState_ChildMergeFunctionality(t *testing.T) { }) t.Run("test write to ledger", func(t *testing.T) { - key := flow.NewRegisterID("address", "key4") + key := flow.NewRegisterID(owner, "key4") value := createByteArray(4) // set key4 on parent err := st.Set(key, value) @@ -138,7 +141,7 @@ func TestExecutionState_MaxValueSize(t *testing.T) { nil, state.DefaultParameters().WithMaxValueSizeAllowed(6)) - key := flow.NewRegisterID("address", "key") + key := flow.NewRegisterID(unittest.RandomAddressFixture(), "key") // update should pass value := createByteArray(5) @@ -157,8 +160,8 @@ func TestExecutionState_MaxKeySize(t *testing.T) { // Note: owners are always 8 bytes state.DefaultParameters().WithMaxKeySizeAllowed(8+2)) - key1 := flow.NewRegisterID("1", "23") - key2 := flow.NewRegisterID("123", "234") + key1 := flow.NewRegisterID(unittest.RandomAddressFixture(), "23") + key2 := flow.NewRegisterID(unittest.RandomAddressFixture(), "234") // read _, err := st.Get(key1) @@ -179,19 +182,19 @@ func TestExecutionState_MaxKeySize(t *testing.T) { } func TestExecutionState_MaxInteraction(t *testing.T) { - key1 := flow.NewRegisterID("1", "2") + key1 := flow.NewRegisterID(unittest.RandomAddressFixture(), "2") key1Size := uint64(8 + 1) value1 := []byte("A") value1Size := uint64(1) - key2 := flow.NewRegisterID("123", "23") + key2 := flow.NewRegisterID(unittest.RandomAddressFixture(), "23") key2Size := uint64(8 + 2) - key3 := flow.NewRegisterID("234", "345") + key3 := flow.NewRegisterID(unittest.RandomAddressFixture(), "345") key3Size := uint64(8 + 3) - key4 := flow.NewRegisterID("3", "4567") + key4 := flow.NewRegisterID(unittest.RandomAddressFixture(), "4567") key4Size := uint64(8 + 4) st := state.NewExecutionState( diff --git a/fvm/storage/state/spock_state_test.go b/fvm/storage/state/spock_state_test.go index eafd30c1305..b3313493bd9 100644 --- a/fvm/storage/state/spock_state_test.go +++ b/fvm/storage/state/spock_state_test.go @@ -9,10 +9,13 @@ import ( "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/utils/rand" + "github.com/onflow/flow-go/utils/unittest" ) type spockTestOp func(*testing.T, *spockState) +var fooOwner = unittest.RandomAddressFixture() + func chainSpockTestOps(prevOps spockTestOp, op spockTestOp) spockTestOp { return func(t *testing.T, state *spockState) { if prevOps != nil { @@ -50,7 +53,7 @@ func testSpock( } func TestSpockStateGet(t *testing.T) { - registerId := flow.NewRegisterID("foo", "bar") + registerId := flow.NewRegisterID(fooOwner, "bar") states := testSpock( t, @@ -71,11 +74,11 @@ func TestSpockStateGet(t *testing.T) { }, // Reading different register ids will result in different spock func(t *testing.T, state *spockState) { - _, err := state.Get(flow.NewRegisterID("fo0", "bar")) + _, err := state.Get(flow.NewRegisterID(unittest.RandomAddressFixture(), "bar")) require.NoError(t, err) }, func(t *testing.T, state *spockState) { - _, err := state.Get(flow.NewRegisterID("foo", "baR")) + _, err := state.Get(flow.NewRegisterID(fooOwner, "baR")) require.NoError(t, err) }, }) @@ -94,7 +97,7 @@ func TestSpockStateGet(t *testing.T) { } func TestSpockStateGetDifferentUnderlyingStorage(t *testing.T) { - badRegisterId := flow.NewRegisterID("foo", "bad") + badRegisterId := flow.NewRegisterID(fooOwner, "bad") value1 := flow.RegisterValue([]byte("abc")) value2 := flow.RegisterValue([]byte("blah")) @@ -127,7 +130,7 @@ func TestSpockStateGetDifferentUnderlyingStorage(t *testing.T) { } func TestSpockStateGetVsSetNil(t *testing.T) { - registerId := flow.NewRegisterID("foo", "bar") + registerId := flow.NewRegisterID(fooOwner, "bar") _ = testSpock( t, @@ -144,7 +147,7 @@ func TestSpockStateGetVsSetNil(t *testing.T) { } func TestSpockStateSet(t *testing.T) { - registerId := flow.NewRegisterID("foo", "bar") + registerId := flow.NewRegisterID(fooOwner, "bar") value := flow.RegisterValue([]byte("value")) states := testSpock( @@ -166,11 +169,11 @@ func TestSpockStateSet(t *testing.T) { }, // Setting different register id will result in different spock func(t *testing.T, state *spockState) { - err := state.Set(flow.NewRegisterID("foo", "baR"), value) + err := state.Set(flow.NewRegisterID(fooOwner, "baR"), value) require.NoError(t, err) }, func(t *testing.T, state *spockState) { - err := state.Set(flow.NewRegisterID("foO", "bar"), value) + err := state.Set(flow.NewRegisterID(unittest.RandomAddressFixture(), "bar"), value) require.NoError(t, err) }, // Setting different register value will result in different spock @@ -194,8 +197,8 @@ func TestSpockStateSet(t *testing.T) { } func TestSpockStateSetValueInjection(t *testing.T) { - registerId1 := flow.NewRegisterID("foo", "injection") - registerId2 := flow.NewRegisterID("foo", "inject") + registerId1 := flow.NewRegisterID(fooOwner, "injection") + registerId2 := flow.NewRegisterID(fooOwner, "inject") _ = testSpock( t, @@ -213,7 +216,7 @@ func TestSpockStateSetValueInjection(t *testing.T) { func TestSpockStateMerge(t *testing.T) { readSet := map[flow.RegisterID]struct{}{ - flow.NewRegisterID("foo", "bar"): struct{}{}, + flow.NewRegisterID(fooOwner, "bar"): struct{}{}, } states := testSpock( @@ -265,13 +268,13 @@ func TestSpockStateMerge(t *testing.T) { require.ErrorContains(t, err, "cannot Merge on a finalized state") } func TestSpockStateDropChanges(t *testing.T) { - registerId := flow.NewRegisterID("foo", "read") + registerId := flow.NewRegisterID(fooOwner, "read") setup := func(t *testing.T, state *spockState) { _, err := state.Get(registerId) require.NoError(t, err) - err = state.Set(flow.NewRegisterID("foo", "write"), []byte("blah")) + err = state.Set(flow.NewRegisterID(fooOwner, "write"), []byte("blah")) require.NoError(t, err) } @@ -331,7 +334,7 @@ func TestSpockStateRandomOps(t *testing.T) { chain[len(chain)-1], func(t *testing.T, state *spockState) { _, err := state.Get( - flow.NewRegisterID("", fmt.Sprintf("%d", id))) + flow.NewRegisterID(flow.EmptyAddress, fmt.Sprintf("%d", id))) require.NoError(t, err) })) case uint(1): @@ -347,7 +350,7 @@ func TestSpockStateRandomOps(t *testing.T) { chain[len(chain)-1], func(t *testing.T, state *spockState) { err := state.Set( - flow.NewRegisterID("", fmt.Sprintf("%d", id)), + flow.NewRegisterID(flow.EmptyAddress, fmt.Sprintf("%d", id)), []byte(fmt.Sprintf("%d", value))) require.NoError(t, err) })) @@ -383,18 +386,21 @@ func TestSpockStateRandomOps(t *testing.T) { _ = testSpock(t, chain) } func TestSpockStateNewChild(t *testing.T) { - baseRegisterId := flow.NewRegisterID("", "base") + baseRegisterId := flow.NewRegisterID(flow.EmptyAddress, "base") baseValue := flow.RegisterValue([]byte("base")) - parentRegisterId1 := flow.NewRegisterID("parent", "1") + parentOwner := unittest.RandomAddressFixture() + childOwner := unittest.RandomAddressFixture() + + parentRegisterId1 := flow.NewRegisterID(parentOwner, "1") parentValue := flow.RegisterValue([]byte("parent")) - parentRegisterId2 := flow.NewRegisterID("parent", "2") + parentRegisterId2 := flow.NewRegisterID(parentOwner, "2") - childRegisterId1 := flow.NewRegisterID("child", "1") + childRegisterId1 := flow.NewRegisterID(childOwner, "1") childValue := flow.RegisterValue([]byte("child")) - childRegisterId2 := flow.NewRegisterID("child", "2") + childRegisterId2 := flow.NewRegisterID(childOwner, "2") parent := newSpockState( snapshot.MapStorageSnapshot{ diff --git a/fvm/storage/state/storage_state_test.go b/fvm/storage/state/storage_state_test.go index 87ff6a195ac..ab852bd91f5 100644 --- a/fvm/storage/state/storage_state_test.go +++ b/fvm/storage/state/storage_state_test.go @@ -7,13 +7,16 @@ import ( "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" ) func TestStorageStateSet(t *testing.T) { - registerId1 := flow.NewRegisterID("foo", "1") + fooOwner := unittest.RandomAddressFixture() + + registerId1 := flow.NewRegisterID(fooOwner, "1") value1 := flow.RegisterValue([]byte("value1")) - registerId2 := flow.NewRegisterID("foo", "2") + registerId2 := flow.NewRegisterID(fooOwner, "2") value2 := flow.RegisterValue([]byte("value2")) state := newStorageState(nil) @@ -40,13 +43,13 @@ func TestStorageStateSet(t *testing.T) { func TestStorageStateGetFromNilBase(t *testing.T) { state := newStorageState(nil) - value, err := state.Get(flow.NewRegisterID("foo", "bar")) + value, err := state.Get(flow.NewRegisterID(unittest.RandomAddressFixture(), "bar")) require.NoError(t, err) require.Nil(t, value) } func TestStorageStateGetFromBase(t *testing.T) { - registerId := flow.NewRegisterID("", "base") + registerId := flow.NewRegisterID(flow.EmptyAddress, "base") baseValue := flow.RegisterValue([]byte("base")) state := newStorageState( @@ -89,7 +92,7 @@ func TestStorageStateGetFromBase(t *testing.T) { } func TestStorageStateGetFromWriteSet(t *testing.T) { - registerId := flow.NewRegisterID("", "base") + registerId := flow.NewRegisterID(flow.EmptyAddress, "base") expectedValue := flow.RegisterValue([]byte("base")) state := newStorageState(nil) @@ -112,22 +115,25 @@ func TestStorageStateGetFromWriteSet(t *testing.T) { } func TestStorageStateMerge(t *testing.T) { - baseRegisterId := flow.NewRegisterID("", "base") + parentOwner := unittest.RandomAddressFixture() + childOwner := unittest.RandomAddressFixture() + + baseRegisterId := flow.NewRegisterID(flow.EmptyAddress, "base") baseValue := flow.RegisterValue([]byte("base")) - parentRegisterId1 := flow.NewRegisterID("parent", "1") + parentRegisterId1 := flow.NewRegisterID(parentOwner, "1") parentValue := flow.RegisterValue([]byte("parent")) - parentRegisterId2 := flow.NewRegisterID("parent", "2") + parentRegisterId2 := flow.NewRegisterID(parentOwner, "2") - parentRegisterId3 := flow.NewRegisterID("parent", "3") + parentRegisterId3 := flow.NewRegisterID(parentOwner, "3") originalParentValue3 := flow.RegisterValue([]byte("parent value")) updatedParentValue3 := flow.RegisterValue([]byte("child value")) - childRegisterId1 := flow.NewRegisterID("child", "1") + childRegisterId1 := flow.NewRegisterID(childOwner, "1") childValue1 := flow.RegisterValue([]byte("child")) - childRegisterId2 := flow.NewRegisterID("child", "2") + childRegisterId2 := flow.NewRegisterID(childOwner, "2") parent := newStorageState( snapshot.MapStorageSnapshot{ diff --git a/fvm/storage/state/transaction_state_test.go b/fvm/storage/state/transaction_state_test.go index 5f91fe8b4b5..87144f14b0f 100644 --- a/fvm/storage/state/transaction_state_test.go +++ b/fvm/storage/state/transaction_state_test.go @@ -10,6 +10,7 @@ import ( "github.com/onflow/flow-go/fvm/meter" "github.com/onflow/flow-go/fvm/storage/state" "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" ) func newTestTransactionState() state.NestedTransactionPreparer { @@ -50,7 +51,7 @@ func TestUnrestrictedNestedTransactionBasic(t *testing.T) { // Ensure the values are written to the correctly nested state - key := flow.NewRegisterID("address", "key") + key := flow.NewRegisterID(unittest.RandomAddressFixture(), "key") val := createByteArray(2) err = txn.Set(key, val) @@ -173,7 +174,7 @@ func TestParseRestrictedNestedTransactionBasic(t *testing.T) { // Sanity check - key := flow.NewRegisterID("address", "key") + key := flow.NewRegisterID(unittest.RandomAddressFixture(), "key") v, err := restrictedNestedState2.Get(key) require.NoError(t, err) @@ -280,7 +281,7 @@ func TestRestartNestedTransaction(t *testing.T) { id, err := txn.BeginNestedTransaction() require.NoError(t, err) - key := flow.NewRegisterID("address", "key") + key := flow.NewRegisterID(unittest.RandomAddressFixture(), "key") val := createByteArray(2) for i := 0; i < 10; i++ { @@ -332,7 +333,7 @@ func TestRestartNestedTransactionWithInvalidId(t *testing.T) { id, err := txn.BeginNestedTransaction() require.NoError(t, err) - key := flow.NewRegisterID("address", "key") + key := flow.NewRegisterID(unittest.RandomAddressFixture(), "key") val := createByteArray(2) err = txn.Set(key, val) @@ -498,7 +499,7 @@ func TestFinalizeMainTransaction(t *testing.T) { id1, err := txn.BeginNestedTransaction() require.NoError(t, err) - registerId := flow.NewRegisterID("foo", "bar") + registerId := flow.NewRegisterID(unittest.RandomAddressFixture(), "bar") value, err := txn.Get(registerId) require.NoError(t, err) @@ -531,18 +532,21 @@ func TestInterimReadSet(t *testing.T) { // Setup test with a bunch of outstanding nested transaction. - readRegisterId1 := flow.NewRegisterID("read", "1") - readRegisterId2 := flow.NewRegisterID("read", "2") - readRegisterId3 := flow.NewRegisterID("read", "3") - readRegisterId4 := flow.NewRegisterID("read", "4") + readOwner := unittest.RandomAddressFixture() + writeOwner := unittest.RandomAddressFixture() - writeRegisterId1 := flow.NewRegisterID("write", "1") + readRegisterId1 := flow.NewRegisterID(readOwner, "1") + readRegisterId2 := flow.NewRegisterID(readOwner, "2") + readRegisterId3 := flow.NewRegisterID(readOwner, "3") + readRegisterId4 := flow.NewRegisterID(readOwner, "4") + + writeRegisterId1 := flow.NewRegisterID(writeOwner, "1") writeValue1 := flow.RegisterValue([]byte("value1")) - writeRegisterId2 := flow.NewRegisterID("write", "2") + writeRegisterId2 := flow.NewRegisterID(writeOwner, "2") writeValue2 := flow.RegisterValue([]byte("value2")) - writeRegisterId3 := flow.NewRegisterID("write", "3") + writeRegisterId3 := flow.NewRegisterID(writeOwner, "3") writeValue3 := flow.RegisterValue([]byte("value3")) err := txn.Set(writeRegisterId1, writeValue1) diff --git a/fvm/transactionStorageLimiter_test.go b/fvm/transactionStorageLimiter_test.go index b9b2a87ec3a..45b089ff868 100644 --- a/fvm/transactionStorageLimiter_test.go +++ b/fvm/transactionStorageLimiter_test.go @@ -19,8 +19,8 @@ func TestTransactionStorageLimiter(t *testing.T) { owner := flow.HexToAddress("1") executionSnapshot := &snapshot.ExecutionSnapshot{ WriteSet: map[flow.RegisterID]flow.RegisterValue{ - flow.NewRegisterID(string(owner[:]), "a"): flow.RegisterValue("foo"), - flow.NewRegisterID(string(owner[:]), "b"): flow.RegisterValue("bar"), + flow.NewRegisterID(owner, "a"): flow.RegisterValue("foo"), + flow.NewRegisterID(owner, "b"): flow.RegisterValue("bar"), }, } diff --git a/ledger/common/convert/convert.go b/ledger/common/convert/convert.go index d1c9d732570..4285cfd7ac4 100644 --- a/ledger/common/convert/convert.go +++ b/ledger/common/convert/convert.go @@ -28,7 +28,7 @@ func LedgerKeyToRegisterID(key ledger.Key) (flow.RegisterID, error) { } return flow.NewRegisterID( - string(key.KeyParts[0].Value), + flow.BytesToAddress(key.KeyParts[0].Value), string(key.KeyParts[1].Value), ), nil } diff --git a/ledger/common/convert/convert_test.go b/ledger/common/convert/convert_test.go index 286f7544d7e..c425307319e 100644 --- a/ledger/common/convert/convert_test.go +++ b/ledger/common/convert/convert_test.go @@ -8,6 +8,7 @@ import ( "github.com/onflow/flow-go/ledger" "github.com/onflow/flow-go/ledger/common/convert" "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" ) func TestLedgerKeyToRegisterID(t *testing.T) { @@ -24,7 +25,7 @@ func TestLedgerKeyToRegisterID(t *testing.T) { }, } - expectedRegisterID := flow.NewRegisterID("owner", "key") + expectedRegisterID := unittest.RegisterIDFixture() registerID, err := convert.LedgerKeyToRegisterID(key) require.NoError(t, err) require.Equal(t, expectedRegisterID, registerID) @@ -70,7 +71,7 @@ func TestLedgerKeyToRegisterID_Error(t *testing.T) { } func TestRegisterIDToLedgerKey(t *testing.T) { - registerID := flow.NewRegisterID("owner", "key") + registerID := unittest.RegisterIDFixture() expectedKey := ledger.Key{ KeyParts: []ledger.KeyPart{ { @@ -110,20 +111,21 @@ func TestRegisterIDToLedgerKey_Global(t *testing.T) { } func TestPayloadToRegister(t *testing.T) { + expected := unittest.RegisterIDFixture() t.Run("can convert", func(t *testing.T) { value := []byte("value") p := ledger.NewPayload( ledger.NewKey( []ledger.KeyPart{ - ledger.NewKeyPart(convert.KeyPartOwner, []byte("owner")), - ledger.NewKeyPart(convert.KeyPartKey, []byte("key")), + ledger.NewKeyPart(convert.KeyPartOwner, []byte(expected.Owner)), + ledger.NewKeyPart(convert.KeyPartKey, []byte(expected.Key)), }, ), value, ) regID, regValue, err := convert.PayloadToRegister(p) require.NoError(t, err) - require.Equal(t, flow.NewRegisterID("owner", "key"), regID) + require.Equal(t, expected, regID) require.Equal(t, value, regValue) }) @@ -140,7 +142,7 @@ func TestPayloadToRegister(t *testing.T) { ) regID, regValue, err := convert.PayloadToRegister(p) require.NoError(t, err) - require.Equal(t, flow.NewRegisterID("", "uuid"), regID) + require.Equal(t, flow.NewRegisterID(flow.EmptyAddress, "uuid"), regID) require.Equal(t, "", regID.Owner) require.Equal(t, "uuid", regID.Key) require.True(t, regID.IsInternalState()) diff --git a/ledger/partial/ledger_test.go b/ledger/partial/ledger_test.go index ac11b6930ba..209bf707ed0 100644 --- a/ledger/partial/ledger_test.go +++ b/ledger/partial/ledger_test.go @@ -127,7 +127,7 @@ func TestProofsForEmptyRegisters(t *testing.T) { require.NoError(t, err) // Read one register during execution. - registerID := flow.NewRegisterID("b", "nk") + registerID := flow.NewRegisterID(unittest.RandomAddressFixture(), "nk") allKeys := []ledger.Key{ convert.RegisterIDToLedgerKey(registerID), } diff --git a/model/flow/ledger.go b/model/flow/ledger.go index 96c33849f5f..b6824c8a49e 100644 --- a/model/flow/ledger.go +++ b/model/flow/ledger.go @@ -81,18 +81,18 @@ func ContractRegisterID(address Address, contractName string) RegisterID { func CadenceRegisterID(owner []byte, key []byte) RegisterID { return RegisterID{ - Owner: string(BytesToAddress(owner).Bytes()), + Owner: addressToOwner(BytesToAddress(owner)), Key: string(key), } } -func NewRegisterID(owner, key string) RegisterID { +func NewRegisterID(owner Address, key string) RegisterID { // global registers have an empty owner field ownerString := "" // all other registers have the account's address - if len(owner) > 0 { - ownerString = addressToOwner(BytesToAddress([]byte(owner))) + if owner != EmptyAddress { + ownerString = addressToOwner(owner) } return RegisterID{ diff --git a/model/flow/ledger_test.go b/model/flow/ledger_test.go index 0857a8c373c..6caecd69618 100644 --- a/model/flow/ledger_test.go +++ b/model/flow/ledger_test.go @@ -10,6 +10,8 @@ import ( "github.com/stretchr/testify/require" "github.com/onflow/atree" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" ) // this benchmark can run with this command: @@ -20,7 +22,7 @@ var length int func BenchmarkString(b *testing.B) { - r := NewRegisterID("theowner", "123412341234") + r := NewRegisterID(unittest.RandomAddressFixture(), "123412341234") ownerLen := len(r.Owner) @@ -40,7 +42,7 @@ func BenchmarkString(b *testing.B) { func BenchmarkOriginalString(b *testing.B) { - r := NewRegisterID("theowner", "123412341234") + r := NewRegisterID(unittest.RandomAddressFixture(), "123412341234") ret := fmt.Sprintf("%x/%x", r.Owner, r.Key) @@ -48,12 +50,12 @@ func BenchmarkOriginalString(b *testing.B) { } func TestRegisterID_IsInternalState(t *testing.T) { - requireTrue := func(owner string, key string) { + requireTrue := func(owner Address, key string) { id := NewRegisterID(owner, key) require.True(t, id.IsInternalState()) } - requireFalse := func(owner string, key string) { + requireFalse := func(owner Address, key string) { id := NewRegisterID(owner, key) require.False(t, id.IsInternalState()) } @@ -62,23 +64,23 @@ func TestRegisterID_IsInternalState(t *testing.T) { uuid := UUIDRegisterID(byte(i)) if i == 0 { require.Equal(t, uuid.Key, UUIDKeyPrefix) - requireTrue("", UUIDKeyPrefix) + requireTrue(EmptyAddress, UUIDKeyPrefix) } else { require.Equal(t, uuid.Key, fmt.Sprintf("%s_%d", UUIDKeyPrefix, i)) - requireTrue("", fmt.Sprintf("%s_%d", UUIDKeyPrefix, i)) + requireTrue(EmptyAddress, fmt.Sprintf("%s_%d", UUIDKeyPrefix, i)) } require.True(t, uuid.IsInternalState()) } require.True(t, AddressStateRegisterID.IsInternalState()) - requireTrue("", AddressStateKey) - requireFalse("", "other") - requireFalse("Address", UUIDKeyPrefix) - requireFalse("Address", AddressStateKey) - requireTrue("Address", "public_key_12") - requireTrue("Address", ContractNamesKey) - requireTrue("Address", "code.MYCODE") - requireTrue("Address", AccountStatusKey) - requireFalse("Address", "anything else") + requireTrue(EmptyAddress, AddressStateKey) + requireFalse(EmptyAddress, "other") + requireFalse(unittest.RandomAddressFixture(), UUIDKeyPrefix) + requireFalse(unittest.RandomAddressFixture(), AddressStateKey) + requireTrue(unittest.RandomAddressFixture(), "public_key_12") + requireTrue(unittest.RandomAddressFixture(), ContractNamesKey) + requireTrue(unittest.RandomAddressFixture(), "code.MYCODE") + requireTrue(unittest.RandomAddressFixture(), AccountStatusKey) + requireFalse(unittest.RandomAddressFixture(), "anything else") } func TestRegisterID_String(t *testing.T) { @@ -87,7 +89,7 @@ func TestRegisterID_String(t *testing.T) { slabIndex := atree.StorageIndex([8]byte{0, 0, 0, 0, 0, 0, 0, 189}) id := NewRegisterID( - string([]byte{1, 2, 3, 10}), + flow.BytesToAddress([]byte{1, 2, 3, 10}), string(atree.SlabIndexToLedgerKey(slabIndex))) require.False(t, utf8.ValidString(id.Key)) printable := id.String() @@ -96,7 +98,7 @@ func TestRegisterID_String(t *testing.T) { }) t.Run("non slab invalid utf-8", func(t *testing.T) { - id := NewRegisterID("b\xc5y", "a\xc5z") + id := NewRegisterID(flow.BytesToAddress([]byte("b\xc5y")), "a\xc5z") require.False(t, utf8.ValidString(id.Owner)) require.False(t, utf8.ValidString(id.Key)) printable := id.String() @@ -106,7 +108,7 @@ func TestRegisterID_String(t *testing.T) { t.Run("global register", func(t *testing.T) { uuidRegisterID := UUIDRegisterID(0) - id := NewRegisterID(uuidRegisterID.Owner, uuidRegisterID.Key) + id := NewRegisterID(EmptyAddress, uuidRegisterID.Key) require.Equal(t, uuidRegisterID.Owner, id.Owner) require.Equal(t, uuidRegisterID.Key, id.Key) printable := id.String() diff --git a/module/chunks/chunkVerifier_test.go b/module/chunks/chunkVerifier_test.go index e5bfb6f8e22..4ea50cb3fed 100644 --- a/module/chunks/chunkVerifier_test.go +++ b/module/chunks/chunkVerifier_test.go @@ -50,8 +50,8 @@ var eventsList = flow.EventsList{ const computationUsed = uint64(100) -var id0 = flow.NewRegisterID("00", "") -var id5 = flow.NewRegisterID("05", "") +var id0 = flow.NewRegisterID(unittest.RandomAddressFixture(), "") +var id5 = flow.NewRegisterID(unittest.RandomAddressFixture(), "") // the chain we use for this test suite var testChain = flow.Emulator @@ -403,16 +403,13 @@ func blockFixture(collection *flow.Collection) *flow.Block { } func generateStateUpdates(t *testing.T, f *completeLedger.Ledger) (ledger.State, ledger.Proof, *ledger.Update) { - id1 := flow.NewRegisterID("00", "") - id2 := flow.NewRegisterID("05", "") - entries := flow.RegisterEntries{ { - Key: id1, + Key: id0, Value: []byte{'a'}, }, { - Key: id2, + Key: id5, Value: []byte{'b'}, }, } @@ -432,7 +429,7 @@ func generateStateUpdates(t *testing.T, f *completeLedger.Ledger) (ledger.State, entries = flow.RegisterEntries{ { - Key: id2, + Key: id5, Value: []byte{'B'}, }, } diff --git a/module/state_synchronization/indexer/indexer_core_test.go b/module/state_synchronization/indexer/indexer_core_test.go index 7adb771db3e..79c325fd94c 100644 --- a/module/state_synchronization/indexer/indexer_core_test.go +++ b/module/state_synchronization/indexer/indexer_core_test.go @@ -543,7 +543,7 @@ func trieRegistersPayloadComparer(t *testing.T, triePayloads []*ledger.Payload, func TestIndexerIntegration_StoreAndGet(t *testing.T) { regOwner := "f8d6e0586b0a20c7" regKey := "code" - registerID := flow.NewRegisterID(regOwner, regKey) + registerID := flow.NewRegisterID(flow.HexToAddress(regOwner), regKey) db, dbDir := unittest.TempBadgerDB(t) t.Cleanup(func() { diff --git a/storage/badger/operation/interactions_test.go b/storage/badger/operation/interactions_test.go index fd334c3a6b8..b976a2dafd1 100644 --- a/storage/badger/operation/interactions_test.go +++ b/storage/badger/operation/interactions_test.go @@ -18,15 +18,15 @@ func TestStateInteractionsInsertCheckRetrieve(t *testing.T) { unittest.RunWithBadgerDB(t, func(db *badger.DB) { id1 := flow.NewRegisterID( - string([]byte("\x89krg\u007fBN\x1d\xf5\xfb\xb8r\xbc4\xbd\x98ռ\xf1\xd0twU\xbf\x16N\xb4?,\xa0&;")), + flow.BytesToAddress([]byte("\x89krg\u007fBN\x1d\xf5\xfb\xb8r\xbc4\xbd\x98ռ\xf1\xd0twU\xbf\x16N\xb4?,\xa0&;")), "") - id2 := flow.NewRegisterID(string([]byte{2}), "") - id3 := flow.NewRegisterID(string([]byte{3}), "") + id2 := flow.NewRegisterID(flow.BytesToAddress([]byte{2}), "") + id3 := flow.NewRegisterID(flow.BytesToAddress([]byte{3}), "") executionSnapshot := &snapshot.ExecutionSnapshot{ ReadSet: map[flow.RegisterID]struct{}{ - id2: struct{}{}, - id3: struct{}{}, + id2: {}, + id3: {}, }, WriteSet: map[flow.RegisterID]flow.RegisterValue{ id1: []byte("zażółć gęślą jaźń"), @@ -36,7 +36,7 @@ func TestStateInteractionsInsertCheckRetrieve(t *testing.T) { interactions := []*snapshot.ExecutionSnapshot{ executionSnapshot, - &snapshot.ExecutionSnapshot{}, + {}, } blockID := unittest.IdentifierFixture() diff --git a/storage/pebble/registers_test.go b/storage/pebble/registers_test.go index 0e598e0ce45..34fd26ad602 100644 --- a/storage/pebble/registers_test.go +++ b/storage/pebble/registers_test.go @@ -300,12 +300,13 @@ func Benchmark_PayloadStorage(b *testing.B) { require.NoError(b, err) require.NotNil(b, s) - batchSizeKey := flow.NewRegisterID("batch", "size") + owner := unittest.RandomAddressFixture() + batchSizeKey := flow.NewRegisterID(owner, "size") const maxBatchSize = 1024 var totalBatchSize int keyForBatchSize := func(i int) flow.RegisterID { - return flow.NewRegisterID("batch", strconv.Itoa(i)) + return flow.NewRegisterID(owner, strconv.Itoa(i)) } valueForHeightAndKey := func(i, j int) []byte { return []byte(fmt.Sprintf("%d-%d", i, j)) diff --git a/utils/debug/registerCache.go b/utils/debug/registerCache.go index 086be61b250..11e0c801ca7 100644 --- a/utils/debug/registerCache.go +++ b/utils/debug/registerCache.go @@ -97,10 +97,10 @@ func (f *fileRegisterCache) Get(owner, key string) ([]byte, bool) { func (f *fileRegisterCache) Set(owner, key string, value []byte) { valueCopy := make([]byte, len(value)) copy(valueCopy, value) - fmt.Println(hex.EncodeToString([]byte(owner)), hex.EncodeToString([]byte(key)), len(value)) + ownerAddr := flow.BytesToAddress([]byte(owner)) + fmt.Println(ownerAddr.Hex(), hex.EncodeToString([]byte(key)), len(value)) f.data[owner+"~"+key] = flow.RegisterEntry{ - Key: flow.NewRegisterID(hex.EncodeToString([]byte(owner)), - hex.EncodeToString([]byte(key))), + Key: flow.NewRegisterID(ownerAddr, hex.EncodeToString([]byte(key))), Value: flow.RegisterValue(valueCopy), } } diff --git a/utils/unittest/fixtures.go b/utils/unittest/fixtures.go index 73830143a26..648d0778b39 100644 --- a/utils/unittest/fixtures.go +++ b/utils/unittest/fixtures.go @@ -73,9 +73,13 @@ func AddressFixture() flow.Address { } func RandomAddressFixture() flow.Address { + return RandomAddressFixtureForChain(flow.Testnet) +} + +func RandomAddressFixtureForChain(chainID flow.ChainID) flow.Address { // we use a 32-bit index - since the linear address generator uses 45 bits, // this won't error - addr, err := flow.Testnet.Chain().AddressAtIndex(uint64(rand.Uint32())) + addr, err := chainID.Chain().AddressAtIndex(uint64(rand.Uint32())) if err != nil { panic(err) } @@ -1429,10 +1433,7 @@ func TransactionDSLFixture(chain flow.Chain) dsl.Transaction { // RegisterIDFixture returns a RegisterID with a fixed key and owner func RegisterIDFixture() flow.RegisterID { - return flow.RegisterID{ - Owner: "owner", - Key: "key", - } + return flow.NewRegisterID(RandomAddressFixture(), "key") } // VerifiableChunkDataFixture returns a complete verifiable chunk with an From 51219c7008f5d4f43e560806a793a3d4cb82cfff Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Wed, 13 Dec 2023 10:53:34 -0800 Subject: [PATCH 2/4] move ledger_test to test module --- model/flow/ledger_test.go | 46 +++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/model/flow/ledger_test.go b/model/flow/ledger_test.go index 6caecd69618..0836d15e525 100644 --- a/model/flow/ledger_test.go +++ b/model/flow/ledger_test.go @@ -1,4 +1,4 @@ -package flow +package flow_test import ( "encoding/hex" @@ -22,7 +22,7 @@ var length int func BenchmarkString(b *testing.B) { - r := NewRegisterID(unittest.RandomAddressFixture(), "123412341234") + r := flow.NewRegisterID(unittest.RandomAddressFixture(), "123412341234") ownerLen := len(r.Owner) @@ -42,7 +42,7 @@ func BenchmarkString(b *testing.B) { func BenchmarkOriginalString(b *testing.B) { - r := NewRegisterID(unittest.RandomAddressFixture(), "123412341234") + r := flow.NewRegisterID(unittest.RandomAddressFixture(), "123412341234") ret := fmt.Sprintf("%x/%x", r.Owner, r.Key) @@ -50,36 +50,36 @@ func BenchmarkOriginalString(b *testing.B) { } func TestRegisterID_IsInternalState(t *testing.T) { - requireTrue := func(owner Address, key string) { - id := NewRegisterID(owner, key) + requireTrue := func(owner flow.Address, key string) { + id := flow.NewRegisterID(owner, key) require.True(t, id.IsInternalState()) } - requireFalse := func(owner Address, key string) { - id := NewRegisterID(owner, key) + requireFalse := func(owner flow.Address, key string) { + id := flow.NewRegisterID(owner, key) require.False(t, id.IsInternalState()) } for i := 0; i < 256; i++ { - uuid := UUIDRegisterID(byte(i)) + uuid := flow.UUIDRegisterID(byte(i)) if i == 0 { - require.Equal(t, uuid.Key, UUIDKeyPrefix) - requireTrue(EmptyAddress, UUIDKeyPrefix) + require.Equal(t, uuid.Key, flow.UUIDKeyPrefix) + requireTrue(flow.EmptyAddress, flow.UUIDKeyPrefix) } else { - require.Equal(t, uuid.Key, fmt.Sprintf("%s_%d", UUIDKeyPrefix, i)) - requireTrue(EmptyAddress, fmt.Sprintf("%s_%d", UUIDKeyPrefix, i)) + require.Equal(t, uuid.Key, fmt.Sprintf("%s_%d", flow.UUIDKeyPrefix, i)) + requireTrue(flow.EmptyAddress, fmt.Sprintf("%s_%d", flow.UUIDKeyPrefix, i)) } require.True(t, uuid.IsInternalState()) } - require.True(t, AddressStateRegisterID.IsInternalState()) - requireTrue(EmptyAddress, AddressStateKey) - requireFalse(EmptyAddress, "other") - requireFalse(unittest.RandomAddressFixture(), UUIDKeyPrefix) - requireFalse(unittest.RandomAddressFixture(), AddressStateKey) + require.True(t, flow.AddressStateRegisterID.IsInternalState()) + requireTrue(flow.EmptyAddress, flow.AddressStateKey) + requireFalse(flow.EmptyAddress, "other") + requireFalse(unittest.RandomAddressFixture(), flow.UUIDKeyPrefix) + requireFalse(unittest.RandomAddressFixture(), flow.AddressStateKey) requireTrue(unittest.RandomAddressFixture(), "public_key_12") - requireTrue(unittest.RandomAddressFixture(), ContractNamesKey) + requireTrue(unittest.RandomAddressFixture(), flow.ContractNamesKey) requireTrue(unittest.RandomAddressFixture(), "code.MYCODE") - requireTrue(unittest.RandomAddressFixture(), AccountStatusKey) + requireTrue(unittest.RandomAddressFixture(), flow.AccountStatusKey) requireFalse(unittest.RandomAddressFixture(), "anything else") } @@ -88,7 +88,7 @@ func TestRegisterID_String(t *testing.T) { // slab with 189 should result in \\xbd slabIndex := atree.StorageIndex([8]byte{0, 0, 0, 0, 0, 0, 0, 189}) - id := NewRegisterID( + id := flow.NewRegisterID( flow.BytesToAddress([]byte{1, 2, 3, 10}), string(atree.SlabIndexToLedgerKey(slabIndex))) require.False(t, utf8.ValidString(id.Key)) @@ -98,7 +98,7 @@ func TestRegisterID_String(t *testing.T) { }) t.Run("non slab invalid utf-8", func(t *testing.T) { - id := NewRegisterID(flow.BytesToAddress([]byte("b\xc5y")), "a\xc5z") + id := flow.NewRegisterID(flow.BytesToAddress([]byte("b\xc5y")), "a\xc5z") require.False(t, utf8.ValidString(id.Owner)) require.False(t, utf8.ValidString(id.Key)) printable := id.String() @@ -107,8 +107,8 @@ func TestRegisterID_String(t *testing.T) { }) t.Run("global register", func(t *testing.T) { - uuidRegisterID := UUIDRegisterID(0) - id := NewRegisterID(EmptyAddress, uuidRegisterID.Key) + uuidRegisterID := flow.UUIDRegisterID(0) + id := flow.NewRegisterID(flow.EmptyAddress, uuidRegisterID.Key) require.Equal(t, uuidRegisterID.Owner, id.Owner) require.Equal(t, uuidRegisterID.Key, id.Key) printable := id.String() From 9591a14d5c96ecc8cbbd3480920a1d1b7d29a621 Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Wed, 13 Dec 2023 12:37:49 -0800 Subject: [PATCH 3/4] fix lint --- model/flow/ledger_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/model/flow/ledger_test.go b/model/flow/ledger_test.go index 0836d15e525..b287c3d3bb0 100644 --- a/model/flow/ledger_test.go +++ b/model/flow/ledger_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "github.com/onflow/atree" + "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/utils/unittest" ) From d02cf69e9a913f8bebbc87c6f495618f347abbd6 Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Wed, 13 Dec 2023 14:09:56 -0800 Subject: [PATCH 4/4] fix unittests --- .../backend/backend_executiondata_test.go | 9 ++++++--- fvm/storage/state/spock_state_test.go | 6 ++++-- ledger/common/convert/convert_test.go | 7 ++++--- .../indexer/indexer_core_test.go | 11 ++++++----- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/engine/access/state_stream/backend/backend_executiondata_test.go b/engine/access/state_stream/backend/backend_executiondata_test.go index 51a9da291f3..7bed6ea55da 100644 --- a/engine/access/state_stream/backend/backend_executiondata_test.go +++ b/engine/access/state_stream/backend/backend_executiondata_test.go @@ -61,6 +61,7 @@ type BackendExecutionDataSuite struct { blockMap map[uint64]*flow.Block sealMap map[flow.Identifier]*flow.Seal resultMap map[flow.Identifier]*flow.ExecutionResult + registerID flow.RegisterID } func TestBackendExecutionDataSuite(t *testing.T) { @@ -149,6 +150,8 @@ func (s *BackendExecutionDataSuite) SetupTest() { s.T().Logf("adding exec data for block %d %d %v => %v", i, block.Header.Height, block.ID(), result.ExecutionDataID) } + s.registerID = unittest.RegisterIDFixture() + s.registersAsync = execution.NewRegistersAsyncStore() s.registers = storagemock.NewRegisterIndex(s.T()) err = s.registersAsync.InitDataAvailable(s.registers) @@ -157,7 +160,7 @@ func (s *BackendExecutionDataSuite) SetupTest() { s.registers.On("FirstHeight").Return(rootBlock.Header.Height).Maybe() s.registers.On("Get", mock.AnythingOfType("RegisterID"), mock.AnythingOfType("uint64")).Return( func(id flow.RegisterID, height uint64) (flow.RegisterValue, error) { - if id == unittest.RegisterIDFixture() { + if id == s.registerID { return flow.RegisterValue{}, nil } return nil, storage.ErrNotFound @@ -440,13 +443,13 @@ func (s *BackendExecutionDataSuite) TestSubscribeExecutionDataHandlesErrors() { func (s *BackendExecutionDataSuite) TestGetRegisterValuesErrors() { s.Run("normal case", func() { - res, err := s.backend.GetRegisterValues(flow.RegisterIDs{unittest.RegisterIDFixture()}, s.backend.rootBlockHeight) + res, err := s.backend.GetRegisterValues(flow.RegisterIDs{s.registerID}, s.backend.rootBlockHeight) require.NoError(s.T(), err) require.NotEmpty(s.T(), res) }) s.Run("returns error if block height is out of range", func() { - _, err := s.backend.GetRegisterValues(flow.RegisterIDs{unittest.RegisterIDFixture()}, s.backend.rootBlockHeight+1) + _, err := s.backend.GetRegisterValues(flow.RegisterIDs{s.registerID}, s.backend.rootBlockHeight+1) require.Equal(s.T(), codes.OutOfRange, status.Code(err)) }) diff --git a/fvm/storage/state/spock_state_test.go b/fvm/storage/state/spock_state_test.go index b3313493bd9..c5aed7a9f02 100644 --- a/fvm/storage/state/spock_state_test.go +++ b/fvm/storage/state/spock_state_test.go @@ -53,6 +53,7 @@ func testSpock( } func TestSpockStateGet(t *testing.T) { + otherOwner := unittest.RandomAddressFixture() registerId := flow.NewRegisterID(fooOwner, "bar") states := testSpock( @@ -74,7 +75,7 @@ func TestSpockStateGet(t *testing.T) { }, // Reading different register ids will result in different spock func(t *testing.T, state *spockState) { - _, err := state.Get(flow.NewRegisterID(unittest.RandomAddressFixture(), "bar")) + _, err := state.Get(flow.NewRegisterID(otherOwner, "bar")) require.NoError(t, err) }, func(t *testing.T, state *spockState) { @@ -147,6 +148,7 @@ func TestSpockStateGetVsSetNil(t *testing.T) { } func TestSpockStateSet(t *testing.T) { + otherOwner := unittest.RandomAddressFixture() registerId := flow.NewRegisterID(fooOwner, "bar") value := flow.RegisterValue([]byte("value")) @@ -173,7 +175,7 @@ func TestSpockStateSet(t *testing.T) { require.NoError(t, err) }, func(t *testing.T, state *spockState) { - err := state.Set(flow.NewRegisterID(unittest.RandomAddressFixture(), "bar"), value) + err := state.Set(flow.NewRegisterID(otherOwner, "bar"), value) require.NoError(t, err) }, // Setting different register value will result in different spock diff --git a/ledger/common/convert/convert_test.go b/ledger/common/convert/convert_test.go index c425307319e..55900aa7bc6 100644 --- a/ledger/common/convert/convert_test.go +++ b/ledger/common/convert/convert_test.go @@ -12,11 +12,13 @@ import ( ) func TestLedgerKeyToRegisterID(t *testing.T) { + expectedRegisterID := unittest.RegisterIDFixture() + key := ledger.Key{ KeyParts: []ledger.KeyPart{ { Type: convert.KeyPartOwner, - Value: []byte("owner"), + Value: []byte(expectedRegisterID.Owner), }, { Type: convert.KeyPartKey, @@ -25,7 +27,6 @@ func TestLedgerKeyToRegisterID(t *testing.T) { }, } - expectedRegisterID := unittest.RegisterIDFixture() registerID, err := convert.LedgerKeyToRegisterID(key) require.NoError(t, err) require.Equal(t, expectedRegisterID, registerID) @@ -78,7 +79,7 @@ func TestRegisterIDToLedgerKey(t *testing.T) { Type: convert.KeyPartOwner, // Note: the owner field is extended to address length during NewRegisterID // so we have to do the same here - Value: flow.BytesToAddress([]byte("owner")).Bytes(), + Value: []byte(registerID.Owner), }, { Type: convert.KeyPartKey, diff --git a/module/state_synchronization/indexer/indexer_core_test.go b/module/state_synchronization/indexer/indexer_core_test.go index 79c325fd94c..e4de36f892d 100644 --- a/module/state_synchronization/indexer/indexer_core_test.go +++ b/module/state_synchronization/indexer/indexer_core_test.go @@ -541,9 +541,10 @@ func trieRegistersPayloadComparer(t *testing.T, triePayloads []*ledger.Payload, } func TestIndexerIntegration_StoreAndGet(t *testing.T) { - regOwner := "f8d6e0586b0a20c7" + regOwnerAddress := unittest.RandomAddressFixture() + regOwner := string(regOwnerAddress.Bytes()) regKey := "code" - registerID := flow.NewRegisterID(flow.HexToAddress(regOwner), regKey) + registerID := flow.NewRegisterID(regOwnerAddress, regKey) db, dbDir := unittest.TempBadgerDB(t) t.Cleanup(func() { @@ -561,13 +562,13 @@ func TestIndexerIntegration_StoreAndGet(t *testing.T) { values := [][]byte{[]byte("1"), []byte("1"), []byte("2"), []byte("3"), []byte("4")} for i, val := range values { - testDesc := fmt.Sprintf("test itteration number %d failed with test value %s", i, val) + testDesc := fmt.Sprintf("test iteration number %d failed with test value %s", i, val) height := uint64(i + 1) err := storeRegisterWithValue(index, height, regOwner, regKey, val) - assert.NoError(t, err) + require.NoError(t, err) results, err := index.RegisterValue(registerID, height) - require.Nil(t, err, testDesc) + require.NoError(t, err, testDesc) assert.Equal(t, val, results) } })