Merge pull request #6297 from Algo-devops-service/relstable4.0.3

Author: algojohnlee

.github/workflows/codegen_verification.yml

@@ -6,7 +6,7 @@ on:
   pull_request:
 jobs:
   codegen_verification:
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-22.04
     services:
       converter:
         image: swaggerapi/swagger-converter@sha256:dcfd1c2537f5f271cb4ec942d08aa59ca41b9a24078040061a772afca7e548ae # v1.0.4

Makefile

@@ -20,12 +20,13 @@ else
 endif
 S3_RELEASE_BUCKET = $$S3_RELEASE_BUCKET
 
-GOLANG_VERSIONS				:= $(shell ./scripts/get_golang_version.sh all)
-GOLANG_VERSION_BUILD		:= $(firstword $(GOLANG_VERSIONS))
-GOLANG_VERSION_SUPPORT		:= $(lastword $(GOLANG_VERSIONS))
-GOLANG_VERSION_BUILD_MAJOR	:= $(shell echo $(GOLANG_VERSION_BUILD) | cut -d'.' -f1,2)
-CURRENT_GO_VERSION			:= $(shell go version | cut -d " " -f 3 | tr -d 'go')
-CURRENT_GO_VERSION_MAJOR	:= $(shell echo $(CURRENT_GO_VERSION) | cut -d'.' -f1,2)
+GOLANG_VERSIONS            := $(shell ./scripts/get_golang_version.sh all)
+GOLANG_VERSION_BUILD       := $(firstword $(GOLANG_VERSIONS))
+GOLANG_VERSION_BUILD_MAJOR := $(shell echo $(GOLANG_VERSION_BUILD) | cut -d'.' -f1,2)
+GOLANG_VERSION_MIN         := $(lastword $(GOLANG_VERSIONS))
+GOLANG_VERSION_SUPPORT     := $(shell echo $(GOLANG_VERSION_MIN) | cut -d'.' -f1,2)
+CURRENT_GO_VERSION         := $(shell go version | cut -d " " -f 3 | tr -d 'go')
+CURRENT_GO_VERSION_MAJOR   := $(shell echo $(CURRENT_GO_VERSION) | cut -d'.' -f1,2)
 
 # If build number already set, use it - to ensure same build number across multiple platforms being built
 BUILDNUMBER      ?= $(shell ./scripts/compute_build_number.sh)

buildnumber.dat

@@ -1 +1 @@
-2
+3

ledger/acctdeltas.go

@@ -1032,6 +1032,14 @@ func onlineAccountsNewRoundImpl(
 						prevAcct = updated
 					}
 				} else {
+					if prevAcct.AccountData.IsVotingEmpty() && newAcct.IsVotingEmpty() {
+						// if both old and new are offline, ignore
+						// otherwise the following could happen:
+						// 1. there are multiple offline account deltas so all of them could be inserted
+						// 2. delta.oldAcct is often pulled from a cache that is only updated on new rows insert so
+						// it could pull a very old already deleted offline value resulting one more insert
+						continue
+					}
 					// "delete" by inserting a zero entry
 					var ref trackerdb.OnlineAccountRef
 					ref, err = writer.InsertOnlineAccount(data.address, 0, trackerdb.BaseOnlineAccountData{}, updRound, 0)

ledger/acctdeltas_test.go

@@ -3442,3 +3442,160 @@ func TestEncodedBaseResourceSize(t *testing.T) {
 	require.Less(t, len(encodedAsset), len(encodedApp))
 	require.GreaterOrEqual(t, MaxEncodedBaseResourceDataSize, len(encodedApp))
 }
+
+// TestOnlineAccountsExceedOfflineRows checks for extra rows for offline accounts in online accounts table:
+// 1. Account is online
+// 2. Account goes offline and recorded in baseOnlineAccounts cache
+// 3. Many (>320 normally) rounds later, account gets deleted by prunning
+// 4. Account updated with a transfer
+// 5. Since it is still in baseOnlineAccounts, it fetched as offline and a new offline row is inserted
+// ==> 5 <== could lead to a ghost row in online accounts table that:
+//   - are not needed but still correct
+//   - make catchpoint generation inconsistent across nodes since it content depends on dynamic baseOnlineAccounts cache.
+//
+// 6. A similar behavior is exposed when there are multiple offline updates in a batch with the same result
+// of extra unnesesary rows in the online accounts table.
+func TestOnlineAccountsExceedOfflineRows(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	dbs, _ := storetesting.DbOpenTest(t, true)
+	storetesting.SetDbLogging(t, dbs)
+	defer dbs.Close()
+
+	tx, err := dbs.Wdb.Handle.Begin()
+	require.NoError(t, err)
+	defer tx.Rollback()
+
+	proto := config.Consensus[protocol.ConsensusCurrentVersion]
+
+	var accts map[basics.Address]basics.AccountData
+	sqlitedriver.AccountsInitTest(t, tx, accts, protocol.ConsensusCurrentVersion)
+
+	addrA := ledgertesting.RandomAddress()
+
+	// acct A is new, offline and then online => exercise new entry for account
+	deltaA := onlineAccountDelta{
+		address: addrA,
+		newAcct: []trackerdb.BaseOnlineAccountData{
+			{
+				MicroAlgos:     basics.MicroAlgos{Raw: 100_000_000},
+				BaseVotingData: trackerdb.BaseVotingData{VoteFirstValid: 1, VoteLastValid: 5},
+			},
+			{
+				MicroAlgos: basics.MicroAlgos{Raw: 100_000_000},
+			},
+		},
+		updRound:  []uint64{1, 2},
+		newStatus: []basics.Status{basics.Online, basics.Offline},
+	}
+	updates := compactOnlineAccountDeltas{}
+	updates.deltas = append(updates.deltas, deltaA)
+	writer, err := sqlitedriver.MakeOnlineAccountsSQLWriter(tx, updates.len() > 0)
+	require.NoError(t, err)
+	defer writer.Close()
+
+	lastUpdateRound := basics.Round(2)
+	updated, err := onlineAccountsNewRoundImpl(writer, updates, proto, lastUpdateRound)
+	require.NoError(t, err)
+	require.Len(t, updated, 2)
+
+	var baseOnlineAccounts lruOnlineAccounts
+	baseOnlineAccounts.init(logging.TestingLog(t), 1000, 800)
+	for _, persistedAcct := range updated {
+		baseOnlineAccounts.write(persistedAcct)
+	}
+
+	// make sure baseOnlineAccounts has the entry
+	entry, has := baseOnlineAccounts.read(addrA)
+	require.True(t, has)
+	require.True(t, entry.AccountData.IsVotingEmpty())
+	require.Equal(t, basics.Round(2), entry.UpdRound)
+
+	queries, err := sqlitedriver.OnlineAccountsInitDbQueries(tx)
+	require.NoError(t, err)
+
+	// make sure both rows are in the db
+	history, _, err := queries.LookupOnlineHistory(addrA)
+	require.NoError(t, err)
+	require.Len(t, history, 2)
+	// ASC ordered by updRound
+	require.False(t, history[0].AccountData.IsVotingEmpty())
+	require.Equal(t, basics.Round(1), history[0].UpdRound)
+	require.True(t, history[1].AccountData.IsVotingEmpty())
+	require.Equal(t, basics.Round(2), history[1].UpdRound)
+
+	// test case 1
+	// simulate compact online delta construction with baseOnlineAccounts use
+	acctDelta := ledgercore.AccountDeltas{}
+	ad := ledgercore.AccountData{
+		AccountBaseData: ledgercore.AccountBaseData{
+			Status:     basics.Offline,
+			MicroAlgos: basics.MicroAlgos{Raw: 100_000_000 - 1},
+		},
+	}
+	acctDelta.Upsert(addrA, ad)
+	deltas := []ledgercore.AccountDeltas{acctDelta}
+	updates = makeCompactOnlineAccountDeltas(deltas, 3, baseOnlineAccounts)
+
+	// make sure old is filled from baseOnlineAccounts
+	require.Empty(t, updates.misses)
+	require.Len(t, updates.deltas, 1)
+	require.NotEmpty(t, updates.deltas[0].oldAcct)
+	require.True(t, updates.deltas[0].oldAcct.AccountData.IsVotingEmpty())
+	require.Equal(t, 1, updates.deltas[0].nOnlineAcctDeltas)
+	require.Equal(t, basics.Offline, updates.deltas[0].newStatus[0])
+	require.True(t, updates.deltas[0].newAcct[0].IsVotingEmpty())
+
+	// insert and make sure no new rows are inserted
+	lastUpdateRound = basics.Round(3)
+	updated, err = onlineAccountsNewRoundImpl(writer, updates, proto, lastUpdateRound)
+	require.NoError(t, err)
+	require.Len(t, updated, 0)
+
+	history, _, err = queries.LookupOnlineHistory(addrA)
+	require.NoError(t, err)
+	require.Len(t, history, 2)
+
+	// test case 2
+	// multiple offline entries in a single batch
+
+	addrB := ledgertesting.RandomAddress()
+
+	// acct A is new, offline and then online => exercise new entry for account
+	deltaB := onlineAccountDelta{
+		address: addrB,
+		newAcct: []trackerdb.BaseOnlineAccountData{
+			{
+				MicroAlgos:     basics.MicroAlgos{Raw: 100_000_000},
+				BaseVotingData: trackerdb.BaseVotingData{VoteFirstValid: 1, VoteLastValid: 5},
+			},
+			{
+				MicroAlgos: basics.MicroAlgos{Raw: 100_000_000},
+			},
+			{
+				MicroAlgos: basics.MicroAlgos{Raw: 100_000_000 - 1},
+			},
+		},
+		updRound:  []uint64{4, 5, 6},
+		newStatus: []basics.Status{basics.Online, basics.Offline, basics.Offline},
+	}
+	updates = compactOnlineAccountDeltas{}
+	updates.deltas = append(updates.deltas, deltaB)
+
+	lastUpdateRound = basics.Round(4)
+	updated, err = onlineAccountsNewRoundImpl(writer, updates, proto, lastUpdateRound)
+	require.NoError(t, err)
+	require.Len(t, updated, 2) // 3rd update is ignored
+
+	// make sure the last offline entry is ignored
+	history, _, err = queries.LookupOnlineHistory(addrB)
+	require.NoError(t, err)
+	require.Len(t, history, 2)
+
+	// ASC ordered by updRound
+	require.False(t, history[0].AccountData.IsVotingEmpty())
+	require.Equal(t, basics.Round(4), history[0].UpdRound)
+	require.True(t, history[1].AccountData.IsVotingEmpty())
+	require.Equal(t, basics.Round(5), history[1].UpdRound)
+}

ledger/catchpointtracker.go

@@ -326,10 +326,12 @@ func (ct *catchpointTracker) finishFirstStageAfterCrash(dbRound basics.Round, bl
 		return err
 	}
 
-	// pass dbRound+1-maxBalLookback as the onlineAccountsForgetBefore parameter: since we can't be sure whether
+	// pass 0 as the onlineAccountsForgetBefore parameter: since we can't be sure whether
 	// there are more than 320 rounds of history in the online accounts tables, this ensures the catchpoint
 	// will only contain the most recent 320 rounds.
-	onlineAccountsForgetBefore := catchpointLookbackHorizonForNextRound(dbRound, config.Consensus[blockProto])
+	// Inside finishFirstStage, this has the same effect as the voters tracker returning a "lowestRound" of 0,
+	// which causes finishFirstStage to calculate the correct horizon and filter out data older than dbround+1-320.
+	onlineAccountsForgetBefore := basics.Round(0)
 	return ct.finishFirstStage(context.Background(), dbRound, onlineAccountsForgetBefore, blockProto, 0)
 }
 

scripts/check_golang_version.sh

@@ -15,33 +15,31 @@ set -eo pipefail
 read -ra GOLANG_VERSIONS <<< "$(./scripts/get_golang_version.sh all)"
 BUILD_VERSION="${GOLANG_VERSIONS[0]}"
 MIN_VERSION="${GOLANG_VERSIONS[1]}"
-GO_MOD_SUPPORT="${GOLANG_VERSIONS[2]}"
 
 # Get the field "go1.1.1" and then remove the "go" prefix.
 SYSTEM_GOLANG_VERSION=$(go version | awk '{ gsub(/go/, "", $3); print $3 }')
 
 # https://golang.org/doc/go1.11#modules
-if [[ "${SYSTEM_GOLANG_VERSION}" < "$GO_MOD_SUPPORT" ]]; then
-    echo "[$0] ERROR: The version of go on the system (${SYSTEM_GOLANG_VERSION}) is too old and does not support go modules. Please update to at least ${MIN_VERSION}"
+if [[ "$(printf '%s\n' ${SYSTEM_GOLANG_VERSION} 1.11 | sort -V | head -n1)" != "1.11" ]]; then
+    echo "[$0] ERROR: The version of go on the system (${SYSTEM_GOLANG_VERSION}) is too old and does not support go modules. Please update to at least 1.11."
     exit 1
 fi
 
-if [ "$1" == "dev" ]; then
-    if [[ "${SYSTEM_GOLANG_VERSION}" < "${MIN_VERSION}" ]]; then
-        echo "[$0] WARNING: The version of go on the system (${SYSTEM_GOLANG_VERSION}) is below the recommended version (${MIN_VERSION}) and therefore may not build correctly."
+if [[ "$(printf '%s\n' "$SYSTEM_GOLANG_VERSION" "$MIN_VERSION" | sort -V | head -n1)" != "$MIN_VERSION" ]]; then
+    # We are below the minimum version
+    if [ "$1" == "dev" ]; then
+        echo "[$0] WARNING: The version of go on the system (${SYSTEM_GOLANG_VERSION}) is below the recommended minimum version (${MIN_VERSION}) and therefore may not build correctly."
         echo "[$0] Please update to at least ${MIN_VERSION}"
-    fi
-elif [ "$1" == "build" ]; then
-    if [[ "${SYSTEM_GOLANG_VERSION}" < "${MIN_VERSION}" ]]; then
-        echo "[$0] ERROR: The version of go on the system (${SYSTEM_GOLANG_VERSION}) is below the necessary version (${MIN_VERSION}) and therefore will not build correctly."
+    elif [ "$1" == "build" ]; then
+        echo "[$0] ERROR: The version of go on the system (${SYSTEM_GOLANG_VERSION}) is below the necessary minimum version (${MIN_VERSION}) and therefore will not build correctly."
         exit 1
     fi
 else
     # Check to make sure that it matches what is specified in `go.mod`.
-    GOMOD_VERSION=$(go mod edit -print | awk '/^go[ \t]+[0-9]+\.[0-9]+(\.[0-9]+)?[ \t]*$/{print $2}')
+    GOMOD_TOOL_VERSION=$(go mod edit -print | awk '$1 == "toolchain" {sub(/^go/, "", $2); print $2}')
 
-    if [[ ! "${BUILD_VERSION}" =~ ^"${GOMOD_VERSION}" ]]; then
-        echo "[$0] ERROR: go version mismatch, go mod version ${GOMOD_VERSION} does not match required version ${BUILD_VERSION}"
+    if [[ "${BUILD_VERSION}" != "${GOMOD_TOOL_VERSION}" ]]; then
+        echo "[$0] ERROR: go version mismatch, go mod tool version ${GOMOD_TOOL_VERSION} does not match required version ${BUILD_VERSION}"
         exit 1
     else
         echo "${BUILD_VERSION}"

scripts/get_golang_version.sh

@@ -12,12 +12,11 @@
 # build a new image whenever the version number has been changed.
 
 BUILD=1.23.3
- MIN=1.23
- GO_MOD_SUPPORT=1.23
+MIN=$(echo $BUILD | cut -d. -f1-2).0
 
 if [ "$1" = all ]
 then
-    echo $BUILD $MIN $GO_MOD_SUPPORT
+    echo $BUILD $MIN
 elif [ "$1" = dev ]
 then
     echo $MIN