From 8f415561904757e6f17b099355ebe675f0b7db7d Mon Sep 17 00:00:00 2001 From: algonautshant <55754073+algonautshant@users.noreply.github.com> Date: Fri, 2 Jul 2021 19:37:00 -0400 Subject: [PATCH] Fix and enable TestNewAccountCanGoOnlineAndParticipate (#2238) * TestNewAccountCanGoOnlineAndParticipate was failing because the test was not waiting enough to get to the round where the newly funded account's funds will be considered for proposing purposes. It was miscalculating the round that it should wait form. Moreover, the rounds considered to when the account is funded was prone to race conditions. In addition, the test was using WaitForRoundWithTimeout which may be very slower if the current round is already ahead. Instead, now it is using ClientWaitForRound, which does not care about individual rounds delayed. * Addressing review commnets: - fixing a typo - getting exact transaction round for funding the account - testing exact blocks for the proposer - using a single node network instead of two nodes - waiting for exactly one round for the new account to propose and checking that - sending the funds and closing the rich account so there will be no possiblity of that proposing a block --- .../e2e-go/features/multisig/multisig_test.go | 4 +- .../onlineOfflineParticipation_test.go | 81 +++++++++++++------ .../participationRewards_test.go | 4 +- test/framework/fixtures/restClientFixture.go | 41 ++++++---- .../{TwoNodesOneOnline.json => OneNode.json} | 8 +- 5 files changed, 89 insertions(+), 49 deletions(-) rename test/testdata/nettemplates/{TwoNodesOneOnline.json => OneNode.json} (85%) diff --git a/test/e2e-go/features/multisig/multisig_test.go b/test/e2e-go/features/multisig/multisig_test.go index c24b4bc5ff..8a086b89de 100644 --- a/test/e2e-go/features/multisig/multisig_test.go +++ b/test/e2e-go/features/multisig/multisig_test.go @@ -68,7 +68,7 @@ func TestBasicMultisig(t *testing.T) { // fund account with enough Algos to allow for 3 transactions and still keep a minBalance in the account amountToFund := 4*minAcctBalance + 3*minTxnFee curStatus, err := client.Status() - fixture.SendMoneyAndWait(curStatus.LastRound, amountToFund, minTxnFee, fundingAddr, multisigAddr) + fixture.SendMoneyAndWait(curStatus.LastRound, amountToFund, minTxnFee, fundingAddr, multisigAddr, "") // try to transact with 1 of 3 amountToSend := minAcctBalance unsignedTransaction, err := client.ConstructPayment(multisigAddr, addrs[0], minTxnFee, amountToSend, nil, "", [32]byte{}, 0, 0) @@ -193,7 +193,7 @@ func TestDuplicateKeys(t *testing.T) { amountToFund := 3 * minAcctBalance txnFee := minTxnFee curStatus, _ := client.Status() - fixture.SendMoneyAndWait(curStatus.LastRound, amountToFund, txnFee, fundingAddr, multisigAddr) + fixture.SendMoneyAndWait(curStatus.LastRound, amountToFund, txnFee, fundingAddr, multisigAddr, "") // try to transact with "1" signature (though, this is a signature from "every" member of the multisig) amountToSend := minAcctBalance unsignedTransaction, err := client.ConstructPayment(multisigAddr, addrs[0], txnFee, amountToSend, nil, "", [32]byte{}, 0, 0) diff --git a/test/e2e-go/features/participation/onlineOfflineParticipation_test.go b/test/e2e-go/features/participation/onlineOfflineParticipation_test.go index b6e3296ceb..2c822f6f9f 100644 --- a/test/e2e-go/features/participation/onlineOfflineParticipation_test.go +++ b/test/e2e-go/features/participation/onlineOfflineParticipation_test.go @@ -18,12 +18,16 @@ package participation import ( "path/filepath" + "runtime" "testing" + "time" "github.com/stretchr/testify/require" "github.com/algorand/go-algorand/config" "github.com/algorand/go-algorand/data/basics" + "github.com/algorand/go-algorand/protocol" + "github.com/algorand/go-algorand/test/e2e-go/globals" "github.com/algorand/go-algorand/test/framework/fixtures" ) @@ -95,20 +99,45 @@ func waitForAccountToProposeBlock(a *require.Assertions, fixture *fixtures.RestC return false } +// TestNewAccountCanGoOnlineAndParticipate tests two behaviors: +// - When the account does not have enough stake, or after receivning algos, but before the lookback rounds, +// it should not be proposing blocks +// - When the account balance receives enough stake, it should be proposing after lookback rounds func TestNewAccountCanGoOnlineAndParticipate(t *testing.T) { - /*if runtime.GOOS == "darwin" { - t.Skip() - } if testing.Short() { t.Skip() - }*/ - t.Skip() // temporary disable the test since it's failing. + } t.Parallel() a := require.New(fixtures.SynchronizedTest(t)) + // Make the seed lookback shorter, otherwise will wait for 320 rounds + consensus := make(config.ConsensusProtocols) + shortPartKeysProtocol := config.Consensus[protocol.ConsensusCurrentVersion] + shortPartKeysProtocol.ApprovedUpgrades = map[protocol.ConsensusVersion]uint64{} + shortPartKeysProtocol.SeedLookback = 2 + shortPartKeysProtocol.SeedRefreshInterval = 8 + if runtime.GOARCH == "amd64" { + // amd64 platforms are generally quite capable, so accelerate the round times to make the test run faster. + shortPartKeysProtocol.AgreementFilterTimeoutPeriod0 = 1 * time.Second + shortPartKeysProtocol.AgreementFilterTimeout = 1 * time.Second + } + consensus[protocol.ConsensusVersion("shortpartkeysprotocol")] = shortPartKeysProtocol + var fixture fixtures.RestClientFixture - fixture.Setup(t, filepath.Join("nettemplates", "TwoNodesOneOnline.json")) + fixture.SetConsensus(consensus) + fixture.SetupNoStart(t, filepath.Join("nettemplates", "OneNode.json")) + + // update the config file by setting the ParticipationKeysRefreshInterval to 5 second. + nodeDirectory, err := fixture.GetNodeDir("Primary") + a.NoError(err) + cfg, err := config.LoadConfigFromDisk(nodeDirectory) + a.NoError(err) + cfg.ParticipationKeysRefreshInterval = 5 * time.Second + cfg.SaveToDisk(nodeDirectory) + + fixture.Start() + defer fixture.Shutdown() client := fixture.LibGoalClient @@ -129,7 +158,7 @@ func TestNewAccountCanGoOnlineAndParticipate(t *testing.T) { transactionFee := minTxnFee amountToSendInitial := 5 * minAcctBalance - fixture.SendMoneyAndWait(initialRound, amountToSendInitial, transactionFee, richAccount, newAccount) + fixture.SendMoneyAndWait(initialRound, amountToSendInitial, transactionFee, richAccount, newAccount, "") amt, err := client.GetBalance(newAccount) a.NoError(err) nodeStatus, err := client.Status() @@ -145,7 +174,7 @@ func TestNewAccountCanGoOnlineAndParticipate(t *testing.T) { a.NoError(err, "rest client should be able to add participation key to new account") a.Equal(newAccount, partkeyResponse.Parent.String(), "partkey response should echo queried account") // account uses part key to go online - goOnlineTx, err := client.MakeUnsignedGoOnlineTx(newAccount, nil, 0, 0, transactionFee, [32]byte{}) + goOnlineTx, err := client.MakeUnsignedGoOnlineTx(newAccount, &partkeyResponse, 0, 0, transactionFee, [32]byte{}) a.NoError(err, "should be able to make go online tx") a.Equal(newAccount, goOnlineTx.Src().String(), "go online response should echo queried account") onlineTxID, err := client.SignAndBroadcastTransaction(wh, nil, goOnlineTx) @@ -159,35 +188,37 @@ func TestNewAccountCanGoOnlineAndParticipate(t *testing.T) { newAccountStatus, err := client.AccountInformation(newAccount) a.NoError(err, "client should be able to get information about new account") a.Equal(basics.Online.String(), newAccountStatus.Status, "new account should be online") - // account receives almost all of rich account's stake (minus enough to - // keep it over MinBalance), so it will be selected for participation + + // transfer the funds and close to the new account amountToSend := richBalance - 3*transactionFee - amountToSendInitial - minAcctBalance - fixture.SendMoneyAndWait(onlineRound, amountToSend, transactionFee, richAccount, newAccount) + txn := fixture.SendMoneyAndWait(onlineRound, amountToSend, transactionFee, richAccount, newAccount, newAccount) + fundedRound := txn.ConfirmedRound nodeStatus, _ = client.Status() - fundedRound := nodeStatus.LastRound - params, err := client.ConsensusParams(nodeStatus.LastRound) a.NoError(err) - lookbackRound := balanceRound(basics.Round(nodeStatus.LastRound), params) - delta := int64(nodeStatus.LastRound) - int64(lookbackRound) - a.True(delta > 0) + accountProposesStarting := balanceRoundOf(basics.Round(fundedRound), params) // Need to wait for funding to take effect on selection, then we can see if we're participating // Stop before the account should become eligible for selection so we can ensure it wasn't - err = fixture.WaitForRoundWithTimeout(fundedRound + uint64(delta) - 1) + err = fixture.ClientWaitForRound(fixture.AlgodClient, uint64(accountProposesStarting-1), + time.Duration(uint64(globals.MaxTimePerRound)*uint64(accountProposesStarting-1))) a.NoError(err) - blockWasProposed := fixture.VerifyBlockProposed(newAccount, int(delta)-1) - a.False(blockWasProposed, "account should not be selected until BalLookback (round %d) passes", int(delta)-1) + // Check if the account did not propose any blocks up to this round + blockWasProposed := fixture.VerifyBlockProposedRange(newAccount, int(accountProposesStarting)-1, + int(accountProposesStarting)-1) + a.False(blockWasProposed, "account should not be selected until BalLookback (round %d) passes", int(accountProposesStarting-1)) + + // Now wait until the round where the funded account will be used. + err = fixture.ClientWaitForRound(fixture.AlgodClient, uint64(accountProposesStarting), 10*globals.MaxTimePerRound) + a.NoError(err) - // check that account starts participating after a while - proposalWindow := 20 // arbitrary - blockWasProposedByNewAccountRecently := waitForAccountToProposeBlock(a, &fixture, newAccount, proposalWindow) + blockWasProposedByNewAccountRecently := fixture.VerifyBlockProposedRange(newAccount, int(accountProposesStarting), 1) a.True(blockWasProposedByNewAccountRecently, "newly online account should be proposing blocks") } -// helper copied from agreement/selector.go -func balanceRound(r basics.Round, cparams config.ConsensusParams) basics.Round { - return r.SubSaturate(basics.Round(2 * cparams.SeedRefreshInterval * cparams.SeedLookback)) +// Returns the earliest round which will have the balanceRound equal to r +func balanceRoundOf(r basics.Round, cparams config.ConsensusParams) basics.Round { + return basics.Round(2*cparams.SeedRefreshInterval*cparams.SeedLookback) + r } diff --git a/test/e2e-go/features/participation/participationRewards_test.go b/test/e2e-go/features/participation/participationRewards_test.go index 60d837ac7c..b3f554834a 100644 --- a/test/e2e-go/features/participation/participationRewards_test.go +++ b/test/e2e-go/features/participation/participationRewards_test.go @@ -335,7 +335,7 @@ func TestRewardRateRecalculation(t *testing.T) { minFee, minBal, err := fixture.MinFeeAndBalance(curStatus.LastRound) r.NoError(err) deadline := curStatus.LastRound + uint64(5) - fixture.SendMoneyAndWait(deadline, amountToSend, minFee, richAccount.Address, rewardsAccount) + fixture.SendMoneyAndWait(deadline, amountToSend, minFee, richAccount.Address, rewardsAccount, "") blk, err := client.Block(curStatus.LastRound) r.NoError(err) @@ -361,7 +361,7 @@ func TestRewardRateRecalculation(t *testing.T) { curStatus, err = client.Status() r.NoError(err) deadline = curStatus.LastRound + uint64(5) - fixture.SendMoneyAndWait(deadline, amountToSend, minFee, richAccount.Address, rewardsAccount) + fixture.SendMoneyAndWait(deadline, amountToSend, minFee, richAccount.Address, rewardsAccount, "") rewardRecalcRound = rewardRecalcRound + consensusParams.RewardsRateRefreshInterval diff --git a/test/framework/fixtures/restClientFixture.go b/test/framework/fixtures/restClientFixture.go index a27a1a5b1e..06a891b955 100644 --- a/test/framework/fixtures/restClientFixture.go +++ b/test/framework/fixtures/restClientFixture.go @@ -56,6 +56,12 @@ func (f *RestClientFixture) SetupShared(testName string, templateFile string) { f.AlgodClient = f.GetAlgodClientForController(f.NC) } +// Start can be called to start the fixture's network if SetupNoStart() was used. +func (f *RestClientFixture) Start() { + f.LibGoalFixture.Start() + f.AlgodClient = f.GetAlgodClientForController(f.NC) +} + // GetAlgodClientForController returns a RestClient for the specified NodeController func (f *RestClientFixture) GetAlgodClientForController(nc nodecontrol.NodeController) client.RestClient { url, err := nc.ServerURL() @@ -267,36 +273,33 @@ func (f *RestClientFixture) WaitForAllTxnsToConfirm(roundTimeout uint64, txidsAn // SendMoneyAndWait uses the rest client to send money and WaitForTxnConfirmation to wait for the send to confirm // it adds some extra error checking as well -func (f *RestClientFixture) SendMoneyAndWait(curRound, amountToSend, transactionFee uint64, fromAccount, toAccount string) (fundingTxid string) { +func (f *RestClientFixture) SendMoneyAndWait(curRound, amountToSend, transactionFee uint64, fromAccount, toAccount string, closeToAccount string) (txn v1.Transaction) { client := f.LibGoalClient wh, err := client.GetUnencryptedWalletHandle() require.NoError(f.t, err, "client should be able to get unencrypted wallet handle") - fundingTxid = f.SendMoneyAndWaitFromWallet(wh, nil, curRound, amountToSend, transactionFee, fromAccount, toAccount) + txn = f.SendMoneyAndWaitFromWallet(wh, nil, curRound, amountToSend, transactionFee, fromAccount, toAccount, closeToAccount) return } // SendMoneyAndWaitFromWallet is as above, but for a specific wallet -func (f *RestClientFixture) SendMoneyAndWaitFromWallet(walletHandle, walletPassword []byte, curRound, amountToSend, transactionFee uint64, fromAccount, toAccount string) (fundingTxid string) { +func (f *RestClientFixture) SendMoneyAndWaitFromWallet(walletHandle, walletPassword []byte, curRound, amountToSend, transactionFee uint64, fromAccount, toAccount string, closeToAccount string) (txn v1.Transaction) { client := f.LibGoalClient - fundingTx, err := client.SendPaymentFromWallet(walletHandle, walletPassword, fromAccount, toAccount, transactionFee, amountToSend, nil, "", 0, 0) + fundingTx, err := client.SendPaymentFromWallet(walletHandle, walletPassword, fromAccount, toAccount, transactionFee, amountToSend, nil, closeToAccount, 0, 0) require.NoError(f.t, err, "client should be able to send money from rich to poor account") require.NotEmpty(f.t, fundingTx.ID().String(), "transaction ID should not be empty") waitingDeadline := curRound + uint64(5) - _, err = f.WaitForConfirmedTxn(waitingDeadline, fromAccount, fundingTx.ID().String()) + txn, err = f.WaitForConfirmedTxn(waitingDeadline, fromAccount, fundingTx.ID().String()) require.NoError(f.t, err) return } -// VerifyBlockProposed checks the last searchRange blocks to see if any blocks were proposed by address -func (f *RestClientFixture) VerifyBlockProposed(account string, searchRange int) (blockWasProposed bool) { +// VerifyBlockProposedRange checks the rounds starting at fromRounds and moving backwards checking countDownNumRounds rounds if any +// blocks were proposed by address +func (f *RestClientFixture) VerifyBlockProposedRange(account string, fromRound, countDownNumRounds int) (blockWasProposed bool) { c := f.LibGoalClient - currentRound, err := c.CurrentRound() - if err != nil { - require.NoError(f.t, err, "client failed to get the last round") - } - for i := 0; i < searchRange; i++ { - block, err := c.Block(currentRound - uint64(i)) - require.NoError(f.t, err, "client failed to get block %d", currentRound-uint64(i)) + for i := 0; i < countDownNumRounds; i++ { + block, err := c.Block(uint64(fromRound - i)) + require.NoError(f.t, err, "client failed to get block %d", fromRound-i) if block.Proposer == account { blockWasProposed = true break @@ -305,6 +308,16 @@ func (f *RestClientFixture) VerifyBlockProposed(account string, searchRange int) return } +// VerifyBlockProposed checks the last searchRange blocks to see if any blocks were proposed by address +func (f *RestClientFixture) VerifyBlockProposed(account string, searchRange int) (blockWasProposed bool) { + c := f.LibGoalClient + currentRound, err := c.CurrentRound() + if err != nil { + require.NoError(f.t, err, "client failed to get the last round") + } + return f.VerifyBlockProposedRange(account, int(currentRound), int(searchRange)) +} + // GetBalancesOnSameRound gets the balances for the passed addresses, and keeps trying until the balances are all the same round // if it can't get the balances for the same round within maxRetries retries, it will return the last balance seen for each acct // it also returns whether it got balances all for the same round, and what the last queried round was diff --git a/test/testdata/nettemplates/TwoNodesOneOnline.json b/test/testdata/nettemplates/OneNode.json similarity index 85% rename from test/testdata/nettemplates/TwoNodesOneOnline.json rename to test/testdata/nettemplates/OneNode.json index cb9f1c7079..60b3689895 100644 --- a/test/testdata/nettemplates/TwoNodesOneOnline.json +++ b/test/testdata/nettemplates/OneNode.json @@ -1,6 +1,7 @@ { "Genesis": { "NetworkName": "tbd", + "ConsensusProtocol": "shortpartkeysprotocol", "Wallets": [ { "Name": "Rich", @@ -27,12 +28,7 @@ { "Name": "Rich", "ParticipationOnly": false }, { "Name": "Partkey", - "ParticipationOnly": true } - ] - }, - { - "Name": "Node", - "Wallets": [ + "ParticipationOnly": true }, { "Name": "Offline", "ParticipationOnly": true } ]