review feedback

ingest/change.go

@@ -2,6 +2,7 @@ package ingest
 
 import (
 	"bytes"
+	"fmt"
 
 	"github.com/stellar/go/support/errors"
 	"github.com/stellar/go/xdr"
@@ -70,26 +71,32 @@ func GetChangesFromLedgerEntryChanges(ledgerEntryChanges xdr.LedgerEntryChanges)
 
 // GetChangesFromLedgerEntryEvictions transforms evicted LedgerKeys to []Change.
 // The generated changes always remove the entries.
-func GetChangesFromLedgerEntryEvictions(keys []xdr.LedgerKey) []Change {
+func GetChangesFromLedgerEntryEvictions(keys []xdr.LedgerKey) ([]Change, error) {
 	changes := []Change{}
 
 	for _, key := range keys {
 		state := xdr.LedgerEntry{}
 		switch key.Type {
 		case xdr.LedgerEntryTypeContractData:
-			state.Data.SetContractData(&xdr.ContractDataEntry{
+			err := state.Data.SetContractData(&xdr.ContractDataEntry{
 				Contract:   key.ContractData.Contract,
 				Key:        key.ContractData.Key,
 				Durability: key.ContractData.Durability,
 			})
+			if err != nil {
+				return nil, errors.Wrap(err, "error setting ContractDataEntry")
+			}
 		case xdr.LedgerEntryTypeContractCode:
-			state.Data.SetContractCode(&xdr.ContractCodeEntry{
+			err := state.Data.SetContractCode(&xdr.ContractCodeEntry{
 				Hash: key.ContractCode.Hash,
 			})
+			if err != nil {
+				return nil, errors.Wrap(err, "error setting ContractCodeEntry")
+			}
 		default:
 			// Currently only contractData and contractCode are evicted by core, so
 			// we only need to handle those two.
-			panic("Invalid LedgerEntry eviction type")
+			return nil, fmt.Errorf("invalid LedgerEntry eviction type: %s", key.Type)
 		}
 		changes = append(changes, Change{
 			Type: key.Type,
@@ -98,7 +105,7 @@ func GetChangesFromLedgerEntryEvictions(keys []xdr.LedgerKey) []Change {
 		})
 	}
 
-	return changes
+	return changes, nil
 }
 
 // LedgerEntryChangeType returns type in terms of LedgerEntryChangeType.

ingest/ledger_change_reader.go

@@ -126,9 +126,10 @@ func (r *LedgerChangeReader) Read() (Change, error) {
 		return r.Read()
 	case evictionChangesState:
 		// Get contract ledgerEntry evictions
-		changes := GetChangesFromLedgerEntryEvictions(
-			r.ledgerCloseMeta.EvictedLedgerKeys(),
-		)
+		changes, err := GetChangesFromLedgerEntryEvictions(r.ledgerCloseMeta.EvictedLedgerKeys())
+		if err != nil {
+			return Change{}, err
+		}
 		r.pending = append(r.pending, changes...)
 		r.state++
 		return r.Read()

ingest/ledger_change_reader_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/stellar/go/ingest/ledgerbackend"
 	"github.com/stellar/go/network"
@@ -83,31 +84,31 @@ type changePredicate func(*testing.T, int, Change)
 func isBalance(address string, balance int64) changePredicate {
 	return func(t *testing.T, idx int, change Change) {
 		msg := fmt.Sprintf("change %d", idx)
-		assert.NotNil(t, change.Post, msg)
+		require.NotNil(t, change.Post, msg)
 		assert.Equal(t, xdr.LedgerEntryTypeAccount.String(), change.Post.Data.Type.String(), msg)
 		assert.Equal(t, address, change.Post.Data.Account.AccountId.Address(), msg)
-		assert.Equal(t, balance, int64(change.Post.Data.Account.Balance), msg)
+		assert.EqualValues(t, balance, change.Post.Data.Account.Balance, msg)
 	}
 }
 
 func isContractDataExtension(contract xdr.ScAddress, key xdr.ScVal, extension uint32) changePredicate {
 	return func(t *testing.T, idx int, change Change) {
 		msg := fmt.Sprintf("change %d", idx)
-		assert.NotNil(t, change.Post, msg)
-		assert.NotNil(t, change.Pre, msg)
+		require.NotNil(t, change.Post, msg)
+		require.NotNil(t, change.Pre, msg)
 		assert.Equal(t, xdr.LedgerEntryTypeContractData.String(), change.Post.Data.Type.String(), msg)
 		assert.Equal(t, contract, change.Post.Data.ContractData.Contract, msg)
 		assert.Equal(t, key, change.Post.Data.ContractData.Key, msg)
 		newExpiry := change.Post.Data.ContractData.ExpirationLedgerSeq
 		oldExpiry := change.Pre.Data.ContractData.ExpirationLedgerSeq
-		assert.Equal(t, extension, uint32(newExpiry-oldExpiry), msg)
+		assert.EqualValues(t, extension, newExpiry-oldExpiry, msg)
 	}
 }
 
 func isContractDataEviction(contract xdr.ScAddress, key xdr.ScVal) changePredicate {
 	return func(t *testing.T, idx int, change Change) {
 		msg := fmt.Sprintf("change %d", idx)
-		assert.NotNil(t, change.Pre, msg)
+		require.NotNil(t, change.Pre, msg)
 		assert.Nil(t, change.Post, msg)
 		assert.Equal(t, xdr.LedgerEntryTypeContractData.String(), change.Pre.Data.Type.String(), msg)
 		assert.Equal(t, contract, change.Pre.Data.ContractData.Contract, msg)
@@ -388,13 +389,13 @@ func TestLedgerChangeLedgerCloseMetaV2(t *testing.T) {
 	tempKey := xdr.ScSymbol("TEMPKEY")
 	persistentKey := xdr.ScSymbol("TEMPVAL")
 	persistentVal := xdr.ScSymbol("PERSVAL")
-	contractIdBytes, err := hex.DecodeString("df06d62447fd25da07c0135eed7557e5a5497ee7d15b7fe345bd47e191d8f577")
+	contractIDBytes, err := hex.DecodeString("df06d62447fd25da07c0135eed7557e5a5497ee7d15b7fe345bd47e191d8f577")
 	assert.NoError(t, err)
-	var contractId xdr.Hash
-	copy(contractId[:], contractIdBytes)
+	var contractID xdr.Hash
+	copy(contractID[:], contractIDBytes)
 	contractAddress := xdr.ScAddress{
 		Type:       xdr.ScAddressTypeScAddressTypeContract,
-		ContractId: &contractId,
+		ContractId: &contractID,
 	}
 	val := xdr.Uint32(123)
 	ledger := xdr.LedgerCloseMeta{
@@ -483,7 +484,7 @@ func TestLedgerChangeLedgerCloseMetaV2(t *testing.T) {
 													ContractData: &xdr.ContractDataEntry{
 														Contract: xdr.ScAddress{
 															Type:       xdr.ScAddressTypeScAddressTypeContract,
-															ContractId: &contractId,
+															ContractId: &contractID,
 														},
 														Key: xdr.ScVal{
 															Type: xdr.ScValTypeScvSymbol,

xdr/ledger_close_meta.go

@@ -93,9 +93,6 @@ func (l LedgerCloseMeta) TransactionResultPair(i int) TransactionResultPair {
 	case 1:
 		return l.MustV1().TxProcessing[i].Result
 	case 2:
-		if l.MustV2().TxProcessing[i].TxApplyProcessing.V != 3 {
-			panic("TransactionResult unavailable because LedgerCloseMeta.V = 2 and TransactionMeta.V != 3")
-		}
 		return l.MustV2().TxProcessing[i].Result
 	default:
 		panic(fmt.Sprintf("Unsupported LedgerCloseMeta.V: %d", l.V))
@@ -124,6 +121,9 @@ func (l LedgerCloseMeta) TxApplyProcessing(i int) TransactionMeta {
 	case 1:
 		return l.MustV1().TxProcessing[i].TxApplyProcessing
 	case 2:
+		if l.MustV2().TxProcessing[i].TxApplyProcessing.V != 3 {
+			panic("TransactionResult unavailable because LedgerCloseMeta.V = 2 and TransactionMeta.V != 3")
+		}
 		return l.MustV2().TxProcessing[i].TxApplyProcessing
 	default:
 		panic(fmt.Sprintf("Unsupported LedgerCloseMeta.V: %d", l.V))