Fix and enable TestNewAccountCanGoOnlineAndParticipate (algorand#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

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)

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
 }

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
 

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

test/testdata/nettemplates/TwoNodesOneOnline.json → 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 }
       ]