From e6b4e40a643f15646871760cca50994c3fe63020 Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Wed, 18 Sep 2024 15:02:05 -0400 Subject: [PATCH 1/3] Use agreement round when estimating a node's proposal interval --- ledger/eval/eval.go | 31 +++- .../features/incentives/whalejoin_test.go | 146 ++++++++++++++++++ 2 files changed, 172 insertions(+), 5 deletions(-) create mode 100644 test/e2e-go/features/incentives/whalejoin_test.go diff --git a/ledger/eval/eval.go b/ledger/eval/eval.go index 859b62922f..9b9ab3cc61 100644 --- a/ledger/eval/eval.go +++ b/ledger/eval/eval.go @@ -253,7 +253,7 @@ func (x *roundCowBase) onlineStake() (basics.MicroAlgos, error) { return basics.MicroAlgos{}, err } x.totalOnline = total - return x.totalOnline, err + return x.totalOnline, nil } func (x *roundCowBase) updateAssetResourceCache(aa ledgercore.AccountAsset, r ledgercore.AssetResource) { @@ -1619,7 +1619,11 @@ func (eval *BlockEvaluator) generateKnockOfflineAccountsList() { updates := &eval.block.ParticipationUpdates ch := activeChallenge(&eval.proto, uint64(eval.Round()), eval.state) - + onlineStake, err := eval.state.onlineStake() + if err != nil { + logging.Base().Errorf("unable to fetch online stake, no knockoffs: %v", err) + return + } for _, accountAddr := range eval.state.modifiedAccounts() { acctData, found := eval.state.mods.Accts.GetData(accountAddr) if !found { @@ -1647,7 +1651,12 @@ func (eval *BlockEvaluator) generateKnockOfflineAccountsList() { if acctData.Status == basics.Online { lastSeen := max(acctData.LastProposed, acctData.LastHeartbeat) - if isAbsent(eval.state.prevTotals.Online.Money, acctData.MicroAlgos, lastSeen, current) || + oad, lErr := eval.state.lookupAgreement(accountAddr) + if lErr != nil { + logging.Base().Errorf("unable to check account for absenteeism: %v", accountAddr) + continue + } + if isAbsent(onlineStake, oad.VotingStake(), lastSeen, current) || failsChallenge(ch, accountAddr, lastSeen) { updates.AbsentParticipationAccounts = append( updates.AbsentParticipationAccounts, @@ -1692,7 +1701,7 @@ func isAbsent(totalOnlineStake basics.MicroAlgos, acctStake basics.MicroAlgos, l // Don't consider accounts that were online when payouts went into effect as // absent. They get noticed the next time they propose or keyreg, which // ought to be soon, if they are high stake or want to earn incentives. - if lastSeen == 0 { + if lastSeen == 0 || acctStake.Raw == 0 { return false } // See if the account has exceeded 10x their expected observation interval. @@ -1806,6 +1815,14 @@ func (eval *BlockEvaluator) validateAbsentOnlineAccounts() error { addressSet := make(map[basics.Address]bool, suspensionCount) ch := activeChallenge(&eval.proto, uint64(eval.Round()), eval.state) + totalOnlineStake, err := eval.state.onlineStake() + if err != nil { + logging.Base().Errorf("unable to fetch online stake, can't check knockoffs: %v", err) + // I suppose we can still return successfully if the absent list is empty. + if len(eval.block.ParticipationUpdates.AbsentParticipationAccounts) > 0 { + return err + } + } for _, accountAddr := range eval.block.ParticipationUpdates.AbsentParticipationAccounts { if _, exists := addressSet[accountAddr]; exists { @@ -1823,7 +1840,11 @@ func (eval *BlockEvaluator) validateAbsentOnlineAccounts() error { } lastSeen := max(acctData.LastProposed, acctData.LastHeartbeat) - if isAbsent(eval.state.prevTotals.Online.Money, acctData.MicroAlgos, lastSeen, eval.Round()) { + oad, lErr := eval.state.lookupAgreement(accountAddr) + if lErr != nil { + return fmt.Errorf("unable to check absent account: %v", accountAddr) + } + if isAbsent(totalOnlineStake, oad.VotingStake(), lastSeen, eval.Round()) { continue // ok. it's "normal absent" } if failsChallenge(ch, accountAddr, lastSeen) { diff --git a/test/e2e-go/features/incentives/whalejoin_test.go b/test/e2e-go/features/incentives/whalejoin_test.go new file mode 100644 index 0000000000..3f70b4700c --- /dev/null +++ b/test/e2e-go/features/incentives/whalejoin_test.go @@ -0,0 +1,146 @@ +// Copyright (C) 2019-2024 Algorand, Inc. +// This file is part of go-algorand +// +// go-algorand is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// go-algorand is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with go-algorand. If not, see . + +package suspension + +import ( + "fmt" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/algorand/go-algorand/daemon/algod/api/server/v2/generated/model" + "github.com/algorand/go-algorand/data/basics" + "github.com/algorand/go-algorand/data/transactions" + "github.com/algorand/go-algorand/libgoal" + "github.com/algorand/go-algorand/protocol" + "github.com/algorand/go-algorand/test/framework/fixtures" + "github.com/algorand/go-algorand/test/partitiontest" +) + +// TestWhaleJoin shows a "whale" with more stake than is currently online can go +// online without immediate suspension. This tests for a bug we had where we +// calcululated expected proposal interval using the _old_ totals, rather than +// the totals following the keyreg. So big joiner could be expected to propose +// in the same block they joined. +func TestWhaleJoin(t *testing.T) { + partitiontest.PartitionTest(t) + defer fixtures.ShutdownSynchronizedTest(t) + + t.Parallel() + a := require.New(fixtures.SynchronizedTest(t)) + + var fixture fixtures.RestClientFixture + // Make rounds shorter and seed lookback smaller, otherwise we need to wait + // 320 slow rounds for particpation effects to matter. + const lookback = 32 + fixture.FasterConsensus(protocol.ConsensusFuture, time.Second, lookback) + fixture.Setup(t, filepath.Join("nettemplates", "Payouts.json")) + defer fixture.Shutdown() + + // Overview of this test: + // 1. Take wallet15 offline (but retain keys so can back online later) + // 2. Have wallet01 spend almost all their algos + // 3. Wait for balances to flow through "lookback" + // 4. Rejoin wallet15 which will have way more stake that what is online. + + clientAndAccount := func(name string) (libgoal.Client, model.Account) { + c := fixture.GetLibGoalClientForNamedNode(name) + accounts, err := fixture.GetNodeWalletsSortedByBalance(c) + a.NoError(err) + a.Len(accounts, 1) + fmt.Printf("Client %s is %v\n", name, accounts[0].Address) + return c, accounts[0] + } + + c15, account15 := clientAndAccount("Node15") + c01, account01 := clientAndAccount("Node01") + + // 1. take wallet15 offline + keys := offline(&fixture, a, c15, account15.Address) + + // 2. c01 starts with 100M, so burn 99.9M to get total online stake down + burn, err := c01.SendPaymentFromUnencryptedWallet(account01.Address, basics.Address{}.String(), + 1000, 99_900_000_000_000, nil) + a.NoError(err) + receipt, err := fixture.WaitForConfirmedTxn(uint64(burn.LastValid), burn.ID().String()) + a.NoError(err) + + // 3. Wait lookback rounds + _, err = c01.WaitForRound(*receipt.ConfirmedRound + lookback) + a.NoError(err) + + // 4. rejoin, with 1.5B against the paltry 100k that's currently online + online(&fixture, a, c15, account15.Address, keys) +} + +// Go offline, but return the key material so it's easy to go back online +func offline(f *fixtures.RestClientFixture, a *require.Assertions, client libgoal.Client, address string) transactions.KeyregTxnFields { + offTx, err := client.MakeUnsignedGoOfflineTx(address, 0, 0, 100_000, [32]byte{}) + a.NoError(err) + + data, err := client.AccountData(address) + a.NoError(err) + keys := transactions.KeyregTxnFields{ + VotePK: data.VoteID, + SelectionPK: data.SelectionID, + StateProofPK: data.StateProofID, + VoteFirst: data.VoteFirstValid, + VoteLast: data.VoteLastValid, + VoteKeyDilution: data.VoteKeyDilution, + } + + wh, err := client.GetUnencryptedWalletHandle() + a.NoError(err) + onlineTxID, err := client.SignAndBroadcastTransaction(wh, nil, offTx) + a.NoError(err) + txn, err := f.WaitForConfirmedTxn(uint64(offTx.LastValid), onlineTxID) + a.NoError(err) + // sync up with the network + _, err = client.WaitForRound(*txn.ConfirmedRound) + a.NoError(err) + data, err = client.AccountData(address) + a.NoError(err) + a.Equal(basics.Offline, data.Status) + return keys +} + +// Go online with the supplied key material +func online(f *fixtures.RestClientFixture, a *require.Assertions, client libgoal.Client, address string, keys transactions.KeyregTxnFields) { + // sanity check that we start offline + data, err := client.AccountData(address) + a.NoError(err) + a.Equal(basics.Offline, data.Status) + + // make an empty keyreg, we'll copy in the keys + onTx, err := client.MakeUnsignedGoOfflineTx(address, 0, 0, 100_000, [32]byte{}) + a.NoError(err) + + onTx.KeyregTxnFields = keys + wh, err := client.GetUnencryptedWalletHandle() + a.NoError(err) + onlineTxID, err := client.SignAndBroadcastTransaction(wh, nil, onTx) + a.NoError(err) + _, err = f.WaitForConfirmedTxn(uint64(onTx.LastValid), onlineTxID) + a.NoError(err) + data, err = client.AccountData(address) + a.NoError(err) + // Before bug fix, the account would be suspended in the same round of the + // keyreg, so it would not be online. + a.Equal(basics.Online, data.Status) +} From dbc8d0fade8da30911ed11b665125f3f28627c01 Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Tue, 24 Sep 2024 13:50:14 -0400 Subject: [PATCH 2/3] Make sure the 320 lookback time doesn't knock an account off --- ledger/apply/keyreg.go | 4 +- .../features/incentives/whalejoin_test.go | 116 +++++++++++++++++- 2 files changed, 117 insertions(+), 3 deletions(-) diff --git a/ledger/apply/keyreg.go b/ledger/apply/keyreg.go index f5326f8240..8185d38d26 100644 --- a/ledger/apply/keyreg.go +++ b/ledger/apply/keyreg.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" + "github.com/algorand/go-algorand/agreement" "github.com/algorand/go-algorand/data/basics" "github.com/algorand/go-algorand/data/transactions" ) @@ -79,7 +80,8 @@ func Keyreg(keyreg transactions.KeyregTxnFields, header transactions.Header, bal } record.Status = basics.Online if params.Payouts.Enabled { - record.LastHeartbeat = header.FirstValid + lookback := agreement.BalanceRound(round, balances.ConsensusParams()) + record.LastHeartbeat = round + lookback } record.VoteFirstValid = keyreg.VoteFirst record.VoteLastValid = keyreg.VoteLast diff --git a/test/e2e-go/features/incentives/whalejoin_test.go b/test/e2e-go/features/incentives/whalejoin_test.go index 3f70b4700c..b3e8dac479 100644 --- a/test/e2e-go/features/incentives/whalejoin_test.go +++ b/test/e2e-go/features/incentives/whalejoin_test.go @@ -87,6 +87,117 @@ func TestWhaleJoin(t *testing.T) { // 4. rejoin, with 1.5B against the paltry 100k that's currently online online(&fixture, a, c15, account15.Address, keys) + + // 5. wait for agreement balances to kick in (another lookback's worth, plus some slack) + _, err = c01.WaitForRound(*receipt.ConfirmedRound + 2*lookback + 5) + a.NoError(err) + + data, err := c15.AccountData(account15.Address) + a.NoError(err) + a.Equal(basics.Online, data.Status) + + // even after being in the block to "get noticed" + txn, err := c15.SendPaymentFromUnencryptedWallet(account15.Address, basics.Address{}.String(), + 1000, 1, nil) + a.NoError(err) + _, err = fixture.WaitForConfirmedTxn(uint64(txn.LastValid), txn.ID().String()) + a.NoError(err) + data, err = c15.AccountData(account15.Address) + a.NoError(err) + a.Equal(basics.Online, data.Status) +} + +// TestBigJoin shows that even though an account can't vote during the first 320 +// rounds after joining, it is not marked absent because of that gap. This would +// be a problem for "biggish" accounts, that might already be absent after 320 +// rounds of not voting. +func TestBigJoin(t *testing.T) { + partitiontest.PartitionTest(t) + defer fixtures.ShutdownSynchronizedTest(t) + + t.Parallel() + a := require.New(fixtures.SynchronizedTest(t)) + + var fixture fixtures.RestClientFixture + // We need lookback to be fairly long, so that we can have a node join with + // 1/16 stake, and have lookback be long enough to risk absenteeism. + const lookback = 164 // > 160, which is 10x the 1/16th's interval + fixture.FasterConsensus(protocol.ConsensusFuture, time.Second/2, lookback) + fixture.Setup(t, filepath.Join("nettemplates", "Payouts.json")) + defer fixture.Shutdown() + + // Overview of this test: + // 1. Take wallet01 offline (but retain keys so can back online later) + // 2. Wait `lookback` rounds so it can't propose. + // 3. Rejoin wallet01 which will now have 1/16 of the stake + // 4. Wait 160 rounds and ensure node01 does not get knocked offline for being absent + // 5. Wait the rest of lookback to ensure it _still_ does not get knock off. + + clientAndAccount := func(name string) (libgoal.Client, model.Account) { + c := fixture.GetLibGoalClientForNamedNode(name) + accounts, err := fixture.GetNodeWalletsSortedByBalance(c) + a.NoError(err) + a.Len(accounts, 1) + fmt.Printf("Client %s is %v\n", name, accounts[0].Address) + return c, accounts[0] + } + + c01, account01 := clientAndAccount("Node01") + + // 1. take wallet01 offline + keys := offline(&fixture, a, c01, account01.Address) + + // 2. Wait lookback rounds + wait(&fixture, a, lookback) + + // 4. rejoin, with 1/16 of total stake + onRound := online(&fixture, a, c01, account01.Address, keys) + + // 5. wait for enough rounds to pass, during which c01 can't vote, that is + // could get knocked off. + wait(&fixture, a, 161) + data, err := c01.AccountData(account01.Address) + a.NoError(err) + a.Equal(basics.Online, data.Status) + + // 5a. just to be sure, do a zero pay to get it "noticed" + zeroPay(&fixture, a, c01, account01.Address) + data, err = c01.AccountData(account01.Address) + a.NoError(err) + a.Equal(basics.Online, data.Status) + + // 6. Now wait until lookback after onRound (which should just be a couple + // more rounds). Check again, to ensure that once c01 is _really_ + // online/voting, it is still safe for long enough to propose. + a.NoError(fixture.WaitForRoundWithTimeout(onRound + lookback)) + data, err = c01.AccountData(account01.Address) + a.NoError(err) + a.Equal(basics.Online, data.Status) + + zeroPay(&fixture, a, c01, account01.Address) + data, err = c01.AccountData(account01.Address) + a.NoError(err) + a.Equal(basics.Online, data.Status) + + // The node _could_ have gotten lucky and propose in first couple rounds it + // is allowed to propose, so this test is expected to be "flaky" in a + // sense. It would pass about 1/8 of the time, even if we had the problem it + // is looking for. +} + +func wait(f *fixtures.RestClientFixture, a *require.Assertions, count uint64) { + res, err := f.AlgodClient.Status() + a.NoError(err) + round := res.LastRound + count + a.NoError(f.WaitForRoundWithTimeout(round)) +} + +func zeroPay(f *fixtures.RestClientFixture, a *require.Assertions, + c libgoal.Client, address string) { + pay, err := c.SendPaymentFromUnencryptedWallet(address, address, 1000, 0, nil) + a.NoError(err) + _, err = f.WaitForConfirmedTxn(uint64(pay.LastValid), pay.ID().String()) + a.NoError(err) } // Go offline, but return the key material so it's easy to go back online @@ -121,7 +232,7 @@ func offline(f *fixtures.RestClientFixture, a *require.Assertions, client libgoa } // Go online with the supplied key material -func online(f *fixtures.RestClientFixture, a *require.Assertions, client libgoal.Client, address string, keys transactions.KeyregTxnFields) { +func online(f *fixtures.RestClientFixture, a *require.Assertions, client libgoal.Client, address string, keys transactions.KeyregTxnFields) uint64 { // sanity check that we start offline data, err := client.AccountData(address) a.NoError(err) @@ -136,11 +247,12 @@ func online(f *fixtures.RestClientFixture, a *require.Assertions, client libgoal a.NoError(err) onlineTxID, err := client.SignAndBroadcastTransaction(wh, nil, onTx) a.NoError(err) - _, err = f.WaitForConfirmedTxn(uint64(onTx.LastValid), onlineTxID) + receipt, err := f.WaitForConfirmedTxn(uint64(onTx.LastValid), onlineTxID) a.NoError(err) data, err = client.AccountData(address) a.NoError(err) // Before bug fix, the account would be suspended in the same round of the // keyreg, so it would not be online. a.Equal(basics.Online, data.Status) + return *receipt.ConfirmedRound } From 6037b2acbae6a87c82147c153be7f9da03e1f63d Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Tue, 1 Oct 2024 14:04:56 -0400 Subject: [PATCH 3/3] Refactor to `BalanceLookback` method --- agreement/selector.go | 8 +++++++- ledger/apply/keyreg.go | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/agreement/selector.go b/agreement/selector.go index 2d0f980ac3..1496027bd6 100644 --- a/agreement/selector.go +++ b/agreement/selector.go @@ -51,7 +51,13 @@ func (sel selector) CommitteeSize(proto config.ConsensusParams) uint64 { // looking at online stake (and status and key material). It is exported so that // AVM can provide opcodes that return the same data. func BalanceRound(r basics.Round, cparams config.ConsensusParams) basics.Round { - return r.SubSaturate(basics.Round(2 * cparams.SeedRefreshInterval * cparams.SeedLookback)) + return r.SubSaturate(BalanceLookback(cparams)) +} + +// BalanceLookback is how far back agreement looks when considering balances for +// voting stake. +func BalanceLookback(cparams config.ConsensusParams) basics.Round { + return basics.Round(2 * cparams.SeedRefreshInterval * cparams.SeedLookback) } func seedRound(r basics.Round, cparams config.ConsensusParams) basics.Round { diff --git a/ledger/apply/keyreg.go b/ledger/apply/keyreg.go index 8185d38d26..d883618685 100644 --- a/ledger/apply/keyreg.go +++ b/ledger/apply/keyreg.go @@ -80,7 +80,7 @@ func Keyreg(keyreg transactions.KeyregTxnFields, header transactions.Header, bal } record.Status = basics.Online if params.Payouts.Enabled { - lookback := agreement.BalanceRound(round, balances.ConsensusParams()) + lookback := agreement.BalanceLookback(balances.ConsensusParams()) record.LastHeartbeat = round + lookback } record.VoteFirstValid = keyreg.VoteFirst