diff --git a/.github/workflows/codegen_verification.yml b/.github/workflows/codegen_verification.yml index ad90754d10..8ac30df114 100644 --- a/.github/workflows/codegen_verification.yml +++ b/.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 diff --git a/Makefile b/Makefile index 86f4b43845..1958f7088d 100644 --- a/Makefile +++ b/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) diff --git a/buildnumber.dat b/buildnumber.dat index 0cfbf08886..00750edc07 100644 --- a/buildnumber.dat +++ b/buildnumber.dat @@ -1 +1 @@ -2 +3 diff --git a/ledger/acctdeltas.go b/ledger/acctdeltas.go index 4de4d18921..37021f3bfd 100644 --- a/ledger/acctdeltas.go +++ b/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) diff --git a/ledger/acctdeltas_test.go b/ledger/acctdeltas_test.go index 2d1fd9f28f..c13656ffb6 100644 --- a/ledger/acctdeltas_test.go +++ b/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) +} diff --git a/ledger/catchpointtracker.go b/ledger/catchpointtracker.go index 79ed834faa..7b905c6f6e 100644 --- a/ledger/catchpointtracker.go +++ b/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) } diff --git a/scripts/check_golang_version.sh b/scripts/check_golang_version.sh index 70a02d5d23..39a9de7f63 100755 --- a/scripts/check_golang_version.sh +++ b/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}" diff --git a/scripts/get_golang_version.sh b/scripts/get_golang_version.sh index fa93723936..f1d88bbe27 100755 --- a/scripts/get_golang_version.sh +++ b/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