Skip to content

Commit

Permalink
Merge pull request #5144 from onflow/petera/refactor-newregisterid
Browse files Browse the repository at this point in the history
[Ledger] Refactor NewRegisterID to accept address
  • Loading branch information
peterargue authored Dec 14, 2023
2 parents 88e8600 + d02cf69 commit 701c342
Show file tree
Hide file tree
Showing 28 changed files with 208 additions and 175 deletions.
2 changes: 1 addition & 1 deletion cmd/util/ledger/migrations/storage_fees_migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions cmd/util/ledger/migrations/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion cmd/util/ledger/reporters/atree_reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cmd/util/ledger/reporters/storage_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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))
})

Expand Down
2 changes: 1 addition & 1 deletion engine/execution/scripts/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion fvm/environment/accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
22 changes: 11 additions & 11 deletions fvm/environment/accounts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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} {
Expand All @@ -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} {
Expand All @@ -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} {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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))
Expand Down
10 changes: 6 additions & 4 deletions fvm/environment/derived_data_invalidator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
34 changes: 17 additions & 17 deletions fvm/meter/meter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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}

Expand All @@ -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(),
Expand All @@ -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 */)
Expand All @@ -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 */)
Expand All @@ -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 */)
Expand All @@ -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 */)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)

Expand Down
3 changes: 2 additions & 1 deletion fvm/storage/derived/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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] {
Expand Down Expand Up @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions fvm/storage/snapshot/snapshot_tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
Loading

0 comments on commit 701c342

Please sign in to comment.