apply review comments

cmd/util/ledger/migrations/account_storage_migration.go

@@ -25,7 +25,7 @@ func NewAccountStorageMigration(
 			address,
 			payloads,
 			util.RuntimeInterfaceConfig{},
-			snapshot.IndexMapBased,
+			snapshot.SmallChangeSetSnapshot,
 		)
 		if err != nil {
 			return nil, fmt.Errorf("failed to create migrator runtime: %w", err)

cmd/util/ledger/migrations/atree_register_migration.go

@@ -91,7 +91,7 @@ func (m *AtreeRegisterMigrator) MigrateAccount(
 	oldPayloads []*ledger.Payload,
 ) ([]*ledger.Payload, error) {
 	// create all the runtime components we need for the migration
-	mr, err := NewMigratorRuntime(address, oldPayloads, util.RuntimeInterfaceConfig{}, snapshot.MapBased)
+	mr, err := NewMigratorRuntime(address, oldPayloads, util.RuntimeInterfaceConfig{}, snapshot.LargeChangeSetOrReadonlySnapshot)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create migrator runtime: %w", err)
 	}

cmd/util/ledger/migrations/cadence_value_diff_test.go

@@ -175,7 +175,7 @@ func TestDiffCadenceValues(t *testing.T) {
 				address,
 				[]*ledger.Payload{accountStatusPayload},
 				util.RuntimeInterfaceConfig{},
-				snapshot.IndexMapBased,
+				snapshot.LargeChangeSetOrReadonlySnapshot,
 			)
 			require.NoError(t, err)
 
@@ -276,7 +276,7 @@ func TestDiffCadenceValues(t *testing.T) {
 				address,
 				[]*ledger.Payload{accountStatusPayload},
 				util.RuntimeInterfaceConfig{},
-				snapshot.IndexMapBased,
+				snapshot.LargeChangeSetOrReadonlySnapshot,
 			)
 			require.NoError(t, err)
 
@@ -384,7 +384,7 @@ func TestDiffCadenceValues(t *testing.T) {
 				address,
 				[]*ledger.Payload{accountStatusPayload},
 				util.RuntimeInterfaceConfig{},
-				snapshot.IndexMapBased,
+				snapshot.LargeChangeSetOrReadonlySnapshot,
 			)
 			require.NoError(t, err)
 
@@ -503,7 +503,7 @@ func TestDiffCadenceValues(t *testing.T) {
 				address,
 				[]*ledger.Payload{accountStatusPayload},
 				util.RuntimeInterfaceConfig{},
-				snapshot.IndexMapBased,
+				snapshot.LargeChangeSetOrReadonlySnapshot,
 			)
 			require.NoError(t, err)
 
@@ -629,7 +629,7 @@ func createStorageMapPayloads(t *testing.T, address common.Address, domain strin
 		address,
 		[]*ledger.Payload{accountStatusPayload},
 		util.RuntimeInterfaceConfig{},
-		snapshot.IndexMapBased,
+		snapshot.LargeChangeSetOrReadonlySnapshot,
 	)
 	require.NoError(t, err)
 

cmd/util/ledger/migrations/cadence_value_validation.go

@@ -389,7 +389,7 @@ func newReadonlyStorageRuntime(payloads []*ledger.Payload) (
 	*readonlyStorageRuntime,
 	error,
 ) {
-	snapshot, err := migrationSnapshot.NewPayloadSnapshot(payloads, migrationSnapshot.MapBased)
+	snapshot, err := migrationSnapshot.NewPayloadSnapshot(payloads, migrationSnapshot.LargeChangeSetOrReadonlySnapshot)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create payload snapshot: %w", err)
 	}

cmd/util/ledger/migrations/cadence_value_validation_test.go

@@ -58,7 +58,7 @@ func TestValidateCadenceValues(t *testing.T) {
 				address,
 				[]*ledger.Payload{accountStatusPayload},
 				util.RuntimeInterfaceConfig{},
-				snapshot.IndexMapBased,
+				snapshot.LargeChangeSetOrReadonlySnapshot,
 			)
 			require.NoError(t, err)
 
@@ -151,7 +151,7 @@ func createTestPayloads(t *testing.T, address common.Address, domain string) []*
 		address,
 		[]*ledger.Payload{accountStatusPayload},
 		util.RuntimeInterfaceConfig{},
-		snapshot.IndexMapBased,
+		snapshot.LargeChangeSetOrReadonlySnapshot,
 	)
 	require.NoError(t, err)
 

cmd/util/ledger/migrations/cadence_values_migration.go

@@ -100,7 +100,7 @@ func (m *CadenceBaseMigrator) MigrateAccount(
 		address,
 		oldPayloads,
 		m.runtimeInterfaceConfig,
-		snapshot.IndexMapBased,
+		snapshot.SmallChangeSetSnapshot,
 	)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create migrator runtime: %w", err)

cmd/util/ledger/migrations/cadence_values_migration_test.go

@@ -124,7 +124,7 @@ func checkMigratedPayloads(
 		address,
 		newPayloads,
 		util.RuntimeInterfaceConfig{},
-		snapshot.IndexMapBased,
+		snapshot.SmallChangeSetSnapshot,
 	)
 	require.NoError(t, err)
 
@@ -742,7 +742,7 @@ func TestProgramParsingError(t *testing.T) {
 		testAddress,
 		payloads,
 		util.RuntimeInterfaceConfig{},
-		snapshot.IndexMapBased,
+		snapshot.SmallChangeSetSnapshot,
 	)
 	require.NoError(t, err)
 
@@ -885,7 +885,7 @@ func TestCoreContractUsage(t *testing.T) {
 			testAddress,
 			payloads,
 			util.RuntimeInterfaceConfig{},
-			snapshot.IndexMapBased,
+			snapshot.SmallChangeSetSnapshot,
 		)
 		require.NoError(t, err)
 
@@ -973,7 +973,7 @@ func TestCoreContractUsage(t *testing.T) {
 			testAddress,
 			payloads,
 			util.RuntimeInterfaceConfig{},
-			snapshot.IndexMapBased,
+			snapshot.SmallChangeSetSnapshot,
 		)
 		require.NoError(t, err)
 

cmd/util/ledger/migrations/migrator_runtime.go

@@ -36,7 +36,6 @@ func NewMigratorRuntime(
 	*MigratorRuntime,
 	error,
 ) {
-	// TODO: the snapshot type should be a parameter
 	snapshot, err := snapshot.NewPayloadSnapshot(payloads, snapshotType)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create payload snapshot: %w", err)

cmd/util/ledger/migrations/staged_contracts_migration.go

@@ -165,7 +165,7 @@ func (m *StagedContractsMigration) InitMigration(
 		common.Address{},
 		allPayloads,
 		config,
-		snapshot.IndexMapBased)
+		snapshot.SmallChangeSetSnapshot)
 	if err != nil {
 		return err
 	}
@@ -224,7 +224,7 @@ func (m *StagedContractsMigration) collectAndRegisterStagedContractsFromPayloads
 		stagingAccountAddress,
 		stagingAccountPayloads,
 		util.RuntimeInterfaceConfig{},
-		snapshot.IndexMapBased,
+		snapshot.SmallChangeSetSnapshot,
 	)
 	if err != nil {
 		return err

cmd/util/ledger/migrations/staged_contracts_migration_test.go

@@ -533,7 +533,7 @@ func TestStagedContractsMigration(t *testing.T) {
 				stagingAccountAddress,
 				[]*ledger.Payload{accountStatusPayload},
 				util.RuntimeInterfaceConfig{},
-				snapshot.IndexMapBased,
+				snapshot.SmallChangeSetSnapshot,
 			)
 			require.NoError(t, err)
 

cmd/util/ledger/migrations/transaction_migration.go

@@ -27,7 +27,7 @@ func NewTransactionBasedMigration(
 			fvm.WithTransactionFeesEnabled(false))
 		ctx := fvm.NewContext(options...)
 
-		snapshot, err := migrationSnapshot.NewPayloadSnapshot(payloads, migrationSnapshot.IndexMapBased)
+		snapshot, err := migrationSnapshot.NewPayloadSnapshot(payloads, migrationSnapshot.SmallChangeSetSnapshot)
 		if err != nil {
 			return nil, err
 		}

cmd/util/ledger/util/snapshot/payload_snapshot.go

@@ -14,12 +14,14 @@ import (
 type MigrationSnapshotType int
 
 const (
-	// MapBased is more efficient when the number of payloads that are going to change is either 0
+	// LargeChangeSetOrReadonlySnapshot creates a `map[flow.RegisterID]*ledger.Payload` from the payloads
+	// It is more efficient when the number of payloads that are going to change is either 0
 	// or large compared to the total number of payloads (more than 30% of the current number of payloads).
-	MapBased MigrationSnapshotType = iota
-	// IndexMapBased is more efficient when the number of payloads that are going to change is small
+	LargeChangeSetOrReadonlySnapshot MigrationSnapshotType = iota
+	// SmallChangeSetSnapshot creates a map of indexes `map[flow.RegisterID]int` from the payloads array
+	// It is more efficient when the number of payloads that are going to change is small
 	// compared to the total number of payloads (less than 30% of the current number of payloads).
-	IndexMapBased
+	SmallChangeSetSnapshot
 )
 
 type MigrationSnapshot interface {
@@ -41,10 +43,10 @@ func NewPayloadSnapshot(
 	snapshotType MigrationSnapshotType,
 ) (MigrationSnapshot, error) {
 	switch snapshotType {
-	case MapBased:
-		return newIndexMapSnapshot(payloads)
-	case IndexMapBased:
+	case LargeChangeSetOrReadonlySnapshot:
 		return newMapSnapshot(payloads)
+	case SmallChangeSetSnapshot:
+		return newIndexMapSnapshot(payloads)
 	default:
 		// should never happen
 		panic("unknown snapshot type")
@@ -56,7 +58,7 @@ type mapSnapshot struct {
 	Payloads map[flow.RegisterID]*ledger.Payload
 }
 
-var _ MigrationSnapshot = mapSnapshot{}
+var _ MigrationSnapshot = (*mapSnapshot)(nil)
 
 func newMapSnapshot(payloads []*ledger.Payload) (*mapSnapshot, error) {
 	l := &mapSnapshot{
@@ -76,30 +78,30 @@ func newMapSnapshot(payloads []*ledger.Payload) (*mapSnapshot, error) {
 	return l, nil
 }
 
-func (p mapSnapshot) Get(id flow.RegisterID) (flow.RegisterValue, error) {
+func (p *mapSnapshot) Get(id flow.RegisterID) (flow.RegisterValue, error) {
 	value, exists := p.Payloads[id]
 	if !exists {
 		return nil, nil
 	}
 	return value.Value(), nil
 }
 
-func (p mapSnapshot) Exists(id flow.RegisterID) bool {
+func (p *mapSnapshot) Exists(id flow.RegisterID) bool {
 	_, exists := p.Payloads[id]
 	return exists
 }
 
-func (p mapSnapshot) Len() int {
+func (p *mapSnapshot) Len() int {
 	return len(p.Payloads)
 }
 
-func (p mapSnapshot) PayloadMap() map[flow.RegisterID]*ledger.Payload {
+func (p *mapSnapshot) PayloadMap() map[flow.RegisterID]*ledger.Payload {
 	return p.Payloads
 }
 
 // ApplyChangesAndGetNewPayloads applies the given changes to the snapshot and returns the new payloads.
 // the snapshot is destroyed.
-func (p mapSnapshot) ApplyChangesAndGetNewPayloads(
+func (p *mapSnapshot) ApplyChangesAndGetNewPayloads(
 	changes map[flow.RegisterID]flow.RegisterValue,
 	expectedChangeAddresses map[flow.Address]struct{},
 	logger zerolog.Logger,
@@ -147,17 +149,17 @@ func (p mapSnapshot) ApplyChangesAndGetNewPayloads(
 	return newPayloads, nil
 }
 
-type IndexMapSnapshot struct {
+type indexMapSnapshot struct {
 	reverseMap map[flow.RegisterID]int
 	payloads   []*ledger.Payload
 }
 
-var _ MigrationSnapshot = (*IndexMapSnapshot)(nil)
+var _ MigrationSnapshot = (*indexMapSnapshot)(nil)
 
-func newIndexMapSnapshot(payloads []*ledger.Payload) (*IndexMapSnapshot, error) {
+func newIndexMapSnapshot(payloads []*ledger.Payload) (*indexMapSnapshot, error) {
 	payloadsCopy := make([]*ledger.Payload, len(payloads))
 	copy(payloadsCopy, payloads)
-	l := &IndexMapSnapshot{
+	l := &indexMapSnapshot{
 		reverseMap: make(map[flow.RegisterID]int, len(payloads)),
 		payloads:   payloadsCopy,
 	}
@@ -175,24 +177,24 @@ func newIndexMapSnapshot(payloads []*ledger.Payload) (*IndexMapSnapshot, error)
 	return l, nil
 }
 
-func (p *IndexMapSnapshot) Get(id flow.RegisterID) (flow.RegisterValue, error) {
+func (p *indexMapSnapshot) Get(id flow.RegisterID) (flow.RegisterValue, error) {
 	index, exists := p.reverseMap[id]
 	if !exists {
 		return nil, nil
 	}
 	return p.payloads[index].Value(), nil
 }
 
-func (p *IndexMapSnapshot) Exists(id flow.RegisterID) bool {
+func (p *indexMapSnapshot) Exists(id flow.RegisterID) bool {
 	_, exists := p.reverseMap[id]
 	return exists
 }
 
-func (p *IndexMapSnapshot) Len() int {
+func (p *indexMapSnapshot) Len() int {
 	return len(p.payloads)
 }
 
-func (p *IndexMapSnapshot) PayloadMap() map[flow.RegisterID]*ledger.Payload {
+func (p *indexMapSnapshot) PayloadMap() map[flow.RegisterID]*ledger.Payload {
 	result := make(map[flow.RegisterID]*ledger.Payload, len(p.payloads))
 	for id, index := range p.reverseMap {
 		result[id] = p.payloads[index]
@@ -202,7 +204,7 @@ func (p *IndexMapSnapshot) PayloadMap() map[flow.RegisterID]*ledger.Payload {
 
 // ApplyChangesAndGetNewPayloads applies the given changes to the snapshot and returns the new payloads.
 // the snapshot is destroyed.
-func (p *IndexMapSnapshot) ApplyChangesAndGetNewPayloads(
+func (p *indexMapSnapshot) ApplyChangesAndGetNewPayloads(
 	changes map[flow.RegisterID]flow.RegisterValue,
 	expectedChangeAddresses map[flow.Address]struct{},
 	logger zerolog.Logger,