Merge pull request #6201 from onflow/janez/change-key-index-to-uint32

[FVM] Changed key index type to uint32

Author: janezpodhostnik

access/api.go

@@ -44,8 +44,8 @@ type API interface {
 	GetAccountBalanceAtLatestBlock(ctx context.Context, address flow.Address) (uint64, error)
 	GetAccountBalanceAtBlockHeight(ctx context.Context, address flow.Address, height uint64) (uint64, error)
 
-	GetAccountKeyAtLatestBlock(ctx context.Context, address flow.Address, keyIndex uint64) (*flow.AccountPublicKey, error)
-	GetAccountKeyAtBlockHeight(ctx context.Context, address flow.Address, keyIndex uint64, height uint64) (*flow.AccountPublicKey, error)
+	GetAccountKeyAtLatestBlock(ctx context.Context, address flow.Address, keyIndex uint32) (*flow.AccountPublicKey, error)
+	GetAccountKeyAtBlockHeight(ctx context.Context, address flow.Address, keyIndex uint32, height uint64) (*flow.AccountPublicKey, error)
 	GetAccountKeysAtLatestBlock(ctx context.Context, address flow.Address) ([]flow.AccountPublicKey, error)
 	GetAccountKeysAtBlockHeight(ctx context.Context, address flow.Address, height uint64) ([]flow.AccountPublicKey, error)
 

access/errors.go

@@ -64,7 +64,7 @@ func (e InvalidAddressError) Error() string {
 // DuplicatedSignatureError indicates that two signatures havs been provided for a key (combination of account and key index)
 type DuplicatedSignatureError struct {
 	Address  flow.Address
-	KeyIndex uint64
+	KeyIndex uint32
 }
 
 func (e DuplicatedSignatureError) Error() string {

access/handler.go

@@ -661,7 +661,7 @@ func (h *Handler) GetAccountKeyAtLatestBlock(
 		return nil, status.Errorf(codes.InvalidArgument, "invalid address: %v", err)
 	}
 
-	keyByIndex, err := h.api.GetAccountKeyAtLatestBlock(ctx, address, uint64(req.GetIndex()))
+	keyByIndex, err := h.api.GetAccountKeyAtLatestBlock(ctx, address, req.GetIndex())
 	if err != nil {
 		return nil, err
 	}
@@ -739,7 +739,7 @@ func (h *Handler) GetAccountKeyAtBlockHeight(
 		return nil, status.Errorf(codes.InvalidArgument, "invalid address: %v", err)
 	}
 
-	keyByIndex, err := h.api.GetAccountKeyAtBlockHeight(ctx, address, uint64(req.GetIndex()), req.GetBlockHeight())
+	keyByIndex, err := h.api.GetAccountKeyAtBlockHeight(ctx, address, req.GetIndex(), req.GetBlockHeight())
 	if err != nil {
 		return nil, err
 	}

access/legacy/convert/convert.go

@@ -30,7 +30,7 @@ func MessageToTransaction(m *entitiesproto.Transaction, chain flow.Chain) (flow.
 		if err != nil {
 			return *t, err
 		}
-		t.SetProposalKey(proposalAddress, uint64(proposalKey.GetKeyId()), proposalKey.GetSequenceNumber())
+		t.SetProposalKey(proposalAddress, proposalKey.GetKeyId(), proposalKey.GetSequenceNumber())
 	}
 
 	payer := m.GetPayer()
@@ -55,15 +55,15 @@ func MessageToTransaction(m *entitiesproto.Transaction, chain flow.Chain) (flow.
 		if err != nil {
 			return *t, err
 		}
-		t.AddPayloadSignature(addr, uint64(sig.GetKeyId()), sig.GetSignature())
+		t.AddPayloadSignature(addr, sig.GetKeyId(), sig.GetSignature())
 	}
 
 	for _, sig := range m.GetEnvelopeSignatures() {
 		addr, err := convert.Address(sig.GetAddress(), chain)
 		if err != nil {
 			return *t, err
 		}
-		t.AddEnvelopeSignature(addr, uint64(sig.GetKeyId()), sig.GetSignature())
+		t.AddEnvelopeSignature(addr, sig.GetKeyId(), sig.GetSignature())
 	}
 
 	t.SetScript(m.GetScript())

access/mock/api.go

@@ -303,7 +303,7 @@ func (v *TransactionValidator) checkAddresses(tx *flow.TransactionBody) error {
 func (v *TransactionValidator) checkSignatureDuplications(tx *flow.TransactionBody) error {
 	type uniqueKey struct {
 		address flow.Address
-		index   uint64
+		index   uint32
 	}
 	observedSigs := make(map[uniqueKey]bool)
 	for _, sig := range append(tx.PayloadSignatures, tx.EnvelopeSignatures...) {

access/validator.go

@@ -63,7 +63,7 @@ func checkMachineAccountRun(_ *cobra.Command, _ []string) {
 	log.Debug().
 		Str("machine_account_address", machineAccountInfo.Address).
 		Str("machine_account_pub_key", fmt.Sprintf("%x", encodedRuntimeAccountPubKey(machineAccountPrivKey))).
-		Uint("key_index", machineAccountInfo.KeyIndex).
+		Uint32("key_index", machineAccountInfo.KeyIndex).
 		Str("signing_algo", machineAccountInfo.SigningAlgorithm.String()).
 		Str("hash_algo", machineAccountInfo.HashAlgorithm.String()).
 		Msg("read machine account info from disk")

cmd/bootstrap/cmd/check_machine_account.go

@@ -28,7 +28,7 @@ type transactionInContext struct {
 	GasLimit               uint64   `json:"gas_limit"`
 	PayerAddressHex        string   `json:"payer_address_hex"`
 	ProposalKeyAddressHex  string   `json:"proposal_key_address_hex"`
-	ProposalKeyID          uint64   `json:"proposal_key_id"`
+	ProposalKeyID          uint32   `json:"proposal_key_id"`
 	ProposalSequenceNumber uint64   `json:"proposal_sequence_number"`
 	AuthorizersAddressHex  []string `json:"authorizers_address_hex"`
 	EnvelopeSize           int      `json:"envelope_size"`

cmd/util/cmd/exec-data-json-export/transaction_exporter.go

@@ -480,7 +480,7 @@ func newMigrations(
 				log,
 				opts.NWorker,
 				[]migrators.AccountBasedMigration{
-					&migrators.AccountUsageMigration{},
+					migrators.NewAccountUsageMigration(rwf),
 				},
 			),
 		},

cmd/util/cmd/execution-state-extract/execution_state_extract.go

@@ -119,7 +119,7 @@ func TestFixSlabsWithBrokenReferences(t *testing.T) {
 	accountStatusRegisterID := mustDecodeHex("612e73")
 	updatedAccountStatusRegister := ledger.NewPayload(
 		ledger.NewKey([]ledger.KeyPart{ownerKey, {Type: 2, Value: accountStatusRegisterID}}),
-		[]byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x7, 0xcc, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x9, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0},
+		[]byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x7, 0xc8, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x9, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0},
 	)
 
 	expectedNewPayloads := make([]*ledger.Payload, len(oldPayloads))

cmd/util/ledger/migrations/fix_broken_data_migration_test.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/onflow/flow-go/cmd/util/ledger/reporters"
+
 	"github.com/onflow/cadence/runtime/common"
 	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
@@ -14,13 +16,21 @@ import (
 )
 
 // AccountUsageMigration iterates through each payload, and calculate the storage usage
-// and update the accounts status with the updated storage usage
+// and update the accounts status with the updated storage usage. It also upgrades the
+// account status registers to the latest version.
 type AccountUsageMigration struct {
 	log zerolog.Logger
+	rw  reporters.ReportWriter
 }
 
 var _ AccountBasedMigration = &AccountUsageMigration{}
 
+func NewAccountUsageMigration(rw reporters.ReportWriterFactory) *AccountUsageMigration {
+	return &AccountUsageMigration{
+		rw: rw.ReportWriter("account-usage-migration"),
+	}
+}
+
 func (m *AccountUsageMigration) InitMigration(
 	log zerolog.Logger,
 	_ *registers.ByAccount,
@@ -30,8 +40,6 @@ func (m *AccountUsageMigration) InitMigration(
 	return nil
 }
 
-const oldAccountStatusSize = 25
-
 func (m *AccountUsageMigration) Close() error {
 	return nil
 }
@@ -44,7 +52,7 @@ func (m *AccountUsageMigration) MigrateAccount(
 
 	var status *environment.AccountStatus
 	var statusValue []byte
-	actualSize := uint64(0)
+	actualUsed := uint64(0)
 
 	// Find the account status register,
 	// and calculate the storage usage
@@ -60,7 +68,7 @@ func (m *AccountUsageMigration) MigrateAccount(
 			}
 		}
 
-		actualSize += uint64(environment.RegisterSize(
+		actualUsed += uint64(environment.RegisterSize(
 			flow.RegisterID{
 				Owner: owner,
 				Key:   key,
@@ -82,58 +90,57 @@ func (m *AccountUsageMigration) MigrateAccount(
 		log.Error().
 			Str("account", address.HexWithPrefix()).
 			Msgf("could not find account status register")
-		return nil
+		return fmt.Errorf("could not find account status register")
 	}
 
-	isOldVersionOfStatusRegister := len(statusValue) == oldAccountStatusSize
+	// reading the status will upgrade the status to the latest version, so it might
+	// have a different size than the one in the register
+	newStatusValue := status.ToBytes()
+	statusSizeDiff := len(newStatusValue) - len(statusValue)
+
+	// the status size diff should be added to the actual size
+	if statusSizeDiff < 0 {
+		if uint64(-statusSizeDiff) > actualUsed {
+			log.Error().
+				Str("account", address.HexWithPrefix()).
+				Msgf("account storage used would be negative")
+			return fmt.Errorf("account storage used would be negative")
+		}
 
-	same := m.compareUsage(isOldVersionOfStatusRegister, status, actualSize, address)
-	if same {
-		// there is no problem with the usage, return
-		return nil
+		actualUsed = actualUsed - uint64(-statusSizeDiff)
+	} else if statusSizeDiff > 0 {
+		actualUsed = actualUsed + uint64(statusSizeDiff)
 	}
 
-	if isOldVersionOfStatusRegister {
-		// size will grow by 8 bytes because of the on-the-fly
-		// migration of account status in AccountStatusFromBytes
-		actualSize += 8
-	}
+	currentUsed := status.StorageUsed()
 
-	// update storage used
-	status.SetStorageUsed(actualSize)
+	// update storage used if the actual size is different from the size in the status register
+	// or if the status size is different.
+	if actualUsed != currentUsed || statusSizeDiff != 0 {
+		// update storage used
+		status.SetStorageUsed(actualUsed)
 
-	err = accountRegisters.Set(
-		string(address[:]),
-		flow.AccountStatusKey,
-		status.ToBytes(),
-	)
-	if err != nil {
-		return fmt.Errorf("could not update account status register: %w", err)
+		err = accountRegisters.Set(
+			string(address[:]),
+			flow.AccountStatusKey,
+			status.ToBytes(),
+		)
+		if err != nil {
+			return fmt.Errorf("could not update account status register: %w", err)
+		}
+
+		m.rw.Write(accountUsageMigrationReportData{
+			AccountAddress: address,
+			OldStorageUsed: currentUsed,
+			NewStorageUsed: actualUsed,
+		})
 	}
 
 	return nil
 }
 
-func (m *AccountUsageMigration) compareUsage(
-	isOldVersionOfStatusRegister bool,
-	status *environment.AccountStatus,
-	actualSize uint64,
-	address common.Address,
-) bool {
-	oldSize := status.StorageUsed()
-	if isOldVersionOfStatusRegister {
-		// size will be reported as 8 bytes larger than the actual size due to on-the-fly
-		// migration of account status in AccountStatusFromBytes
-		oldSize -= 8
-	}
-
-	if oldSize != actualSize {
-		m.log.Warn().
-			Uint64("old_size", oldSize).
-			Uint64("new_size", actualSize).
-			Str("account", address.HexWithPrefix()).
-			Msg("account storage used usage mismatch")
-		return false
-	}
-	return true
+type accountUsageMigrationReportData struct {
+	AccountAddress common.Address
+	OldStorageUsed uint64
+	NewStorageUsed uint64
 }