From 80b24d5be24a7798234c31aa536777b85f83bd47 Mon Sep 17 00:00:00 2001
From: Pavel Zbitskiy <pavel@algorand.com>
Date: Wed, 2 Aug 2023 15:39:45 -0400
Subject: [PATCH] ledger: increase locks granularity in lookupWithoutRewards

---
 ledger/acctupdates.go      | 24 ++++++++++++++++++++----
 ledger/acctupdates_test.go |  4 ++--
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/ledger/acctupdates.go b/ledger/acctupdates.go
index 0408034ae2..7bc7997828 100644
--- a/ledger/acctupdates.go
+++ b/ledger/acctupdates.go
@@ -1321,15 +1321,21 @@ func (au *accountUpdates) lookupWithoutRewards(rnd basics.Round, addr basics.Add
 			return
 		}
 
+		deltas := au.deltas[:offset]
 		rewardsVersion = au.versions[offset]
 		rewardsLevel = au.roundTotals[offset].RewardsLevel
 
 		// check if we've had this address modified in the past rounds. ( i.e. if it's in the deltas )
 		macct, indeltas := au.accounts[addr]
+		if synchronized {
+			au.accountsMu.RUnlock()
+			needUnlock = false
+		}
+
 		if indeltas {
 			// Check if this is the most recent round, in which case, we can
 			// use a cache of the most recent account state.
-			if offset == uint64(len(au.deltas)) {
+			if offset == uint64(currentDeltaLen) {
 				return macct.data, rnd, rewardsVersion, rewardsLevel, nil
 			}
 			// the account appears in the deltas, but we don't know if it appears in the
@@ -1337,7 +1343,7 @@ func (au *accountUpdates) lookupWithoutRewards(rnd basics.Round, addr basics.Add
 			// backwards to ensure that later updates take priority if present.
 			for offset > 0 {
 				offset--
-				d, ok := au.deltas[offset].Accts.GetData(addr)
+				d, ok := deltas[offset].Accts.GetData(addr)
 				if ok {
 					// the returned validThrough here is not optimal, but it still correct. We could get a more accurate value by scanning
 					// the deltas forward, but this would be time consuming loop, which might not pay off.
@@ -1352,17 +1358,27 @@ func (au *accountUpdates) lookupWithoutRewards(rnd basics.Round, addr basics.Add
 			rnd = currentDbRound + basics.Round(currentDeltaLen)
 		}
 
+		if synchronized {
+			au.accountsMu.RLock()
+			needUnlock = true
+		}
 		// check the baseAccounts -
 		if macct, has := au.baseAccounts.read(addr); has {
+			// there is a chance the db advanced between deltas check and the baseAccounts check
+			// ensure the round requested is still in the db
+			if macct.Round > rnd {
+				return ledgercore.AccountData{}, 0, "", 0, &RoundOffsetError{rnd, macct.Round}
+			}
+
 			// we don't technically need this, since it's already in the baseAccounts, however, writing this over
 			// would ensure that we promote this field.
 			au.baseAccounts.writePending(macct)
 			return macct.AccountData.GetLedgerCoreAccountData(), rnd, rewardsVersion, rewardsLevel, nil
 		}
 
-		// check baseAccoiunts again to see if it does not exist
+		// check baseAccounts again to see if it does not exist
 		if au.baseAccounts.readNotFound(addr) {
-			// it seems the account doesnt exist
+			// it seems the account does not exist
 			return ledgercore.AccountData{}, rnd, rewardsVersion, rewardsLevel, nil
 		}
 
diff --git a/ledger/acctupdates_test.go b/ledger/acctupdates_test.go
index 658cb71330..b11c1ffe4e 100644
--- a/ledger/acctupdates_test.go
+++ b/ledger/acctupdates_test.go
@@ -830,7 +830,7 @@ func testAcctUpdatesUpdatesCorrectness(t *testing.T, cfg config.Local) {
 				fromAccountDataOld, validThrough, err := au.LookupWithoutRewards(i-1, fromAccount)
 				require.NoError(t, err)
 				require.Equal(t, i-1, validThrough)
-				require.Equalf(t, moneyAccountsExpectedAmounts[i-1][j], fromAccountDataOld.MicroAlgos.Raw, "Account index : %d\nRound number : %d", j, i)
+				require.Equalf(t, int(moneyAccountsExpectedAmounts[i-1][j]), int(fromAccountDataOld.MicroAlgos.Raw), "Account index : %d (%s)\nRound number : %d (%d)", j, moneyAccounts[j], i, i-1)
 
 				fromAccountDataNew := fromAccountDataOld
 
@@ -869,7 +869,7 @@ func testAcctUpdatesUpdatesCorrectness(t *testing.T, cfg config.Local) {
 					require.NoError(t, err)
 					require.GreaterOrEqual(t, int64(validThrough), int64(basics.Round(checkRound-uint64(testback))))
 					// if we received no error, we want to make sure the reported amount is correct.
-					require.Equalf(t, moneyAccountsExpectedAmounts[checkRound-uint64(testback)][j], acct.MicroAlgos.Raw, "Account index : %d\nRound number : %d", j, checkRound)
+					require.Equalf(t, int(moneyAccountsExpectedAmounts[checkRound-uint64(testback)][j]), int(acct.MicroAlgos.Raw), "Account index : %d (%s)\nRound number : %d (%d)", j, moneyAccounts[j], checkRound, checkRound-uint64(testback))
 					testback++
 					j--
 				}