Repository: algorand/indexer
Pull request #241 (2020-11-09 23:40:44)

CumulativeRewards account close handling
Added handling for resetting cumulative rewards when an account is closed.


Patch:
diff --git a/Makefile b/Makefile
index 17ea3f8..bda64d4 100644
--- a/Makefile
+++ b/Makefile
@@ -48,6 +48,9 @@ test: idb/mocks/IndexerDb.go
 lint:
 	golint -set_exit_status ./...
 
+fmt:
+	go fmt ./...
+
 migration-tests: test/migrations/postgres_migration_test.sh
 	test/migrations/postgres_migration_test.sh
 
diff --git a/accounting/accounting.go b/accounting/accounting.go
index 330c735..2f8d940 100644
--- a/accounting/accounting.go
+++ b/accounting/accounting.go
@@ -97,6 +97,29 @@ func bytesAreZero(b []byte) bool {
 	return true
 }
 
+func (accounting *State) closeAccount(addr types.Address) {
+	if accounting.AlgoUpdates == nil {
+		accounting.AlgoUpdates = make(map[[32]byte]*idb.AlgoUpdate)
+	}
+
+	update, hasKey := accounting.AlgoUpdates[addr]
+
+	if !hasKey {
+		update = &idb.AlgoUpdate{
+			Balance: 0,
+			Rewards: 0,
+			Closed: true,
+		}
+		accounting.AlgoUpdates[addr] = update
+		return
+	}
+
+	// If the key was there, override the rewards by setting to zero.
+	// The balance will be updated with the delta as usual.
+	update.Rewards = 0
+	update.Closed = true
+}
+
 func (accounting *State) updateRewards(rewardAddr, acctAddr types.Address, amount types.MicroAlgos) {
 	accounting.updateAlgoAndRewards(acctAddr, int64(amount), int64(amount))
 	// Note: rewardAddr is also available as accounting.rewardAddr, but all of the other accounting is done
@@ -295,6 +318,11 @@ func (accounting *State) AddTransaction(txnr *idb.TxnRow) (err error) {
 		if stxn.CloseRewards != 0 {
 			accounting.updateRewards(accounting.rewardAddr, stxn.Txn.CloseRemainderTo, stxn.CloseRewards)
 		}
+
+		// The sender account is being closed.
+		if !stxn.Txn.CloseRemainderTo.IsZero() {
+			accounting.closeAccount(stxn.Txn.Sender)
+		}
 	} else if stxn.Txn.Type == "keyreg" {
 		// see https://github.com/algorand/go-algorand/blob/master/data/transactions/keyreg.go
 		accounting.updateAccountData(stxn.Txn.Sender, "vote", stxn.Txn.VotePK)
diff --git a/idb/dummy.go b/idb/dummy.go
index d7cdc8b..a56c3bc 100644
--- a/idb/dummy.go
+++ b/idb/dummy.go
@@ -473,9 +473,15 @@ type TxnAssetUpdate struct {
 }
 
 // AlgoUpdate is used by the accounting and IndexerDb implementations to share modifications in a block.
+// When the update does not include closing the account, the values are a delta applied to the account.
+// If the update does include closing the account the rewards must be SET directly instead of applying a delta.
 type AlgoUpdate struct {
 	Balance int64
 	Rewards int64
+	// Closed changes the nature of the Rewards field. Balance and Rewards are normally deltas added to the
+	// microalgos and totalRewards columns, but if an account has been Closed then Rewards becomes a new value
+	// that replaces the old value (always zero by current reward logic)
+	Closed  bool
 }
 
 // RoundUpdates is used by the accounting and IndexerDb implementations to share modifications in a block.
@@ -612,4 +618,4 @@ type Health struct {
 type SpecialAccounts struct {
 	FeeSink     types.Address
 	RewardsPool types.Address
-}
\ No newline at end of file
+}
diff --git a/idb/postgres/postgres.go b/idb/postgres/postgres.go
index 71828d8..fa29099 100644
--- a/idb/postgres/postgres.go
+++ b/idb/postgres/postgres.go
@@ -739,10 +739,25 @@ func (db *IndexerDb) CommitRoundAccounting(updates idb.RoundUpdates, round, rewa
 			return fmt.Errorf("prepare update algo, %v", err)
 		}
 		defer setalgo.Close()
+
+		// If the account is closing the cumulative rewards field needs to be specially overwritten.
+		resetalgo, err := tx.Prepare(`INSERT INTO account (addr, microalgos, rewardsbase, rewardsTotal) VALUES ($1, $2, $3, $4) ON CONFLICT (addr) DO UPDATE SET microalgos = account.microalgos + EXCLUDED.microalgos, rewardsbase = EXCLUDED.rewardsbase, rewardsTotal = EXCLUDED.rewardsTotal`)
+		if err != nil {
+			return fmt.Errorf("prepare reset algo, %v", err)
+		}
+		defer resetalgo.Close()
+
 		for addr, delta := range updates.AlgoUpdates {
-			_, err = setalgo.Exec(addr[:], delta.Balance, rewardsBase, delta.Rewards)
-			if err != nil {
-				return fmt.Errorf("update algo, %v", err)
+			if ! delta.Closed {
+				_, err = setalgo.Exec(addr[:], delta.Balance, rewardsBase, delta.Rewards)
+				if err != nil {
+					return fmt.Errorf("update algo, %v", err)
+				}
+			} else {
+				_, err = resetalgo.Exec(addr[:], delta.Balance, rewardsBase, delta.Rewards)
+				if err != nil {
+					return fmt.Errorf("update algo, %v", err)
+				}
 			}
 		}
 	}
diff --git a/idb/postgres/postgres_migrations.go b/idb/postgres/postgres_migrations.go
index 7a58b22..33a4c85 100644
--- a/idb/postgres/postgres_migrations.go
+++ b/idb/postgres/postgres_migrations.go
@@ -388,11 +388,16 @@ func m5accountCumulativeRewardsUpdateAccounts(db *IndexerDb, state *MigrationSta
 				cumulativeRewards += stxn.ApplyData.SenderRewards
 			}
 
+			// When the account is closed rewards reset to zero.
+			if stxn.Txn.Sender == address && !stxn.Txn.CloseRemainderTo.IsZero() {
+				cumulativeRewards = 0
+			}
+
 			if stxn.Txn.Receiver == address {
 				cumulativeRewards += stxn.ApplyData.ReceiverRewards
 			}
 
-			if stxn.Txn.CloseRemainderTo  == address {
+			if stxn.Txn.CloseRemainderTo == address {
 				cumulativeRewards += stxn.ApplyData.CloseRewards
 			}
 		}


Test patch:
diff --git a/accounting/accounting_test.go b/accounting/accounting_test.go
new file mode 100644
index 0000000..5929e7c
--- /dev/null
+++ b/accounting/accounting_test.go
@@ -0,0 +1,168 @@
+package accounting
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/algorand/go-algorand-sdk/encoding/msgpack"
+	"github.com/algorand/go-algorand-sdk/types"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/algorand/indexer/idb"
+	"github.com/algorand/indexer/idb/mocks"
+)
+
+var mainAcct = decodeAddressOrPanic("GJR76Q6OXNZ2CYIVCFCDTJRBAAR6TYEJJENEII3G2U3JH546SPBQA62IFY")
+var accountB = decodeAddressOrPanic("N5T74SANUWLHI6ZWYFQBEB6J2VXBTYUYZNWQB2V26DCF4ARKC7GDUW3IRU")
+var accountC = decodeAddressOrPanic("OKUWMFFEKF4B4D7FRQYBVV3C2SNS54ZO4WZ2MJ3576UYKFDHM5P3AFMRWE")
+var feeAddr = decodeAddressOrPanic("ZROKLZW4GVOK5WQIF2GUR6LHFVEZBMV56BIQEQD4OTIZL2BPSYYUKFBSHM")
+var rewardAddr = decodeAddressOrPanic("4C3S3A5II6AYMEADSW7EVL7JAKVU2ASJMMJAGVUROIJHYMS6B24NCXVEWM")
+
+// open / close are configured
+var openMainStxn, openMain = makePayTxnRowOrPanic(10, 1000, 10234, 0, 111, 1111, 0, accountC, mainAcct, types.ZeroAddress)
+var closeMainToBCStxn, closeMainToBC = makePayTxnRowOrPanic(10, 1000, 1234, 9111, 0, 111, 111, mainAcct, accountC, accountB)
+
+func decodeAddressOrPanic(addr string) types.Address {
+	if addr == "" {
+		return types.ZeroAddress
+	}
+
+	result, err := types.DecodeAddress(addr)
+	if err != nil {
+		panic(fmt.Sprintf("Failed to decode address: '%s'", addr))
+	}
+	return result
+}
+
+func makePayTxnRowOrPanic(round, fee, amt, closeAmt, sendRewards, receiveRewards, closeRewards uint64, sender, receiver, close types.Address) (*types.SignedTxnWithAD, *idb.TxnRow) {
+	txn := types.SignedTxnWithAD{
+		SignedTxn: types.SignedTxn{
+			Txn: types.Transaction{
+				Type: "pay",
+				Header: types.Header{
+					Sender:     sender,
+					Fee:        types.MicroAlgos(fee),
+					FirstValid: types.Round(round),
+					LastValid:  types.Round(round),
+				},
+				PaymentTxnFields: types.PaymentTxnFields{
+					Receiver:         receiver,
+					Amount:           types.MicroAlgos(amt),
+					CloseRemainderTo: close,
+				},
+			},
+		},
+		ApplyData: types.ApplyData{
+			ClosingAmount:   types.MicroAlgos(closeAmt),
+			SenderRewards:   types.MicroAlgos(sendRewards),
+			ReceiverRewards: types.MicroAlgos(receiveRewards),
+			CloseRewards:    types.MicroAlgos(closeRewards),
+		},
+	}
+
+	txnRow := idb.TxnRow{
+		Round:    uint64(txn.Txn.FirstValid),
+		TxnBytes: msgpack.Encode(txn),
+	}
+
+	return &txn, &txnRow
+}
+
+func assertUpdates(t *testing.T, update *idb.AlgoUpdate, closed bool, balance, rewards int64) {
+	assert.Equal(t, closed, update.Closed)
+	assert.Equal(t, balance, update.Balance)
+	assert.Equal(t, rewards, update.Rewards)
+}
+
+func getAccounting() *State {
+	mockIndexer := &mocks.IndexerDb{}
+	accountingState := New(mockIndexer)
+	accountingState.feeAddr = feeAddr
+	accountingState.rewardAddr = rewardAddr
+	accountingState.currentRound = closeMainToBC.Round
+	return accountingState
+}
+
+func getSenderAmounts(txn *types.SignedTxnWithAD) (balance, rewards int64) {
+	balance = -int64(txn.Txn.Amount) + -int64(txn.Txn.Fee) + -int64(txn.ClosingAmount) + int64(txn.SenderRewards)
+	rewards = int64(txn.SenderRewards)
+	return
+}
+
+func getReceiverAmounts(txn *types.SignedTxnWithAD) (balance, rewards int64) {
+	balance = int64(txn.Txn.Amount) + int64(txn.ReceiverRewards)
+	rewards = int64(txn.ReceiverRewards)
+	return
+}
+
+func getCloseAmounts(txn *types.SignedTxnWithAD) (balance, rewards int64) {
+	balance = int64(txn.ClosingAmount) + int64(txn.CloseRewards)
+	rewards = int64(txn.CloseRewards)
+	return
+}
+
+// TestSimplePayment checks that the accounting deltas are correct for a simple single payment.
+func TestSimplePayment(t *testing.T) {
+	var update *idb.AlgoUpdate
+	state := getAccounting()
+	state.AddTransaction(openMain)
+
+	senderBalance, senderRewards := getSenderAmounts(openMainStxn)
+	update = state.AlgoUpdates[openMainStxn.Txn.Sender]
+	assertUpdates(t, update, false, senderBalance, senderRewards)
+
+	receiverBalance, receiverRewards := getReceiverAmounts(openMainStxn)
+	update = state.AlgoUpdates[openMainStxn.Txn.Receiver]
+	assertUpdates(t, update, false, receiverBalance, receiverRewards)
+}
+
+// TestSimpleAccountClose verifies that the accounting state is set correctly for an account being closed.
+func TestSimpleAccountClose(t *testing.T) {
+	state := getAccounting()
+	state.AddTransaction(closeMainToBC)
+
+	senderBalance, _ := getSenderAmounts(closeMainToBCStxn)
+	assertUpdates(t, state.AlgoUpdates[mainAcct], true, senderBalance, 0)
+
+	closeBalance, closeRewards := getCloseAmounts(closeMainToBCStxn)
+	assertUpdates(t, state.AlgoUpdates[closeMainToBCStxn.Txn.CloseRemainderTo], false, closeBalance, closeRewards)
+
+	receiverBalance, receiverRewards := getReceiverAmounts(closeMainToBCStxn)
+	assertUpdates(t, state.AlgoUpdates[closeMainToBCStxn.Txn.Receiver], false, receiverBalance, receiverRewards)
+}
+
+// TestOpenClose verifies that the accounting state is set correctly when an account is opened and closed in the same round.
+func TestOpenClose(t *testing.T) {
+	var update *idb.AlgoUpdate
+	state := getAccounting()
+	state.AddTransaction(openMain)
+	// In order to make sure rewards were actually reset, they need to be non-zero at this point
+	assert.True(t, state.AlgoUpdates[mainAcct].Rewards > 0)
+	state.AddTransaction(closeMainToBC)
+
+	// main account balance should add up to 0, and rewards should be reset to 0
+	update = state.AlgoUpdates[mainAcct]
+	assertUpdates(t, update, true, 0, 0)
+
+	senderBalance, senderRewards := getSenderAmounts(openMainStxn)
+	receiverBalance, receiverRewards := getReceiverAmounts(closeMainToBCStxn)
+	update = state.AlgoUpdates[openMainStxn.Txn.Sender]
+	assertUpdates(t, update, false, senderBalance + receiverBalance, senderRewards + receiverRewards)
+}
+
+// TestCloseOpen verifies that the accounting state is set correctly when an account is closed then reopened in the same round.
+func TestCloseOpen(t *testing.T) {
+	var update *idb.AlgoUpdate
+	state := getAccounting()
+	state.AddTransaction(openMain)
+	state.AddTransaction(closeMainToBC)
+	// In the real world this would be a different transaction, but this is a test.
+	state.AddTransaction(openMain)
+
+	// The account is closed, but account was closed, but then re-opened and somehow accumulated rewards.
+	// This could happen in a real network if the main account was re-opened by another close-to transaction which
+	// sent closeRewards to the main-account. Yikes.
+	receiverBalance, receiverRewards := getReceiverAmounts(openMainStxn)
+	update = state.AlgoUpdates[mainAcct]
+	assertUpdates(t, update, true, receiverBalance, receiverRewards)
+}