Merge pull request #962 from onetechnical/johnlee/beta2.0.13v2.0.13-beta

Johnlee/beta2.0.13

12 files changed, 369 insertions, 10 deletions

diff --git a/buildnumber.dat b/buildnumber.dat
index 48082f72f..b1bd38b62 100644
--- a/buildnumber.dat
+++ b/buildnumber.dat
@@ -1 +1 @@
-12
+13
diff --git a/config/consensus.go b/config/consensus.go
index 1dba9b75b..9c9f25fb1 100644
--- a/config/consensus.go
+++ b/config/consensus.go
@@ -192,7 +192,12 @@ type ConsensusParams struct {
 	MaxTxGroupSize int
 
 	// support for transaction leases
+	// note: if FixTransactionLeases is not set, the transaction
+	// leases supported are faulty; specifically, they do not
+	// enforce exclusion correctly when the FirstValid of
+	// transactions do not match.
 	SupportTransactionLeases bool
+	FixTransactionLeases     bool
 
 	// 0 for no support, otherwise highest version supported
 	LogicSigVersion uint64
@@ -551,9 +556,18 @@ func initConsensusProtocols() {
 	// v21 can be upgraded to v22.
 	v21.ApprovedUpgrades[protocol.ConsensusV22] = 0
 
+	// v23 is an upgrade which fixes the behavior of leases so that
+	// it conforms with the intended spec.
+	v23 := v22
+	v23.ApprovedUpgrades = map[protocol.ConsensusVersion]uint64{}
+	v23.FixTransactionLeases = true
+	Consensus[protocol.ConsensusV23] = v23
+	// v22 can be upgraded to v23.
+	v22.ApprovedUpgrades[protocol.ConsensusV23] = 10000
+
 	// ConsensusFuture is used to test features that are implemented
 	// but not yet released in a production protocol version.
-	vFuture := v22
+	vFuture := v23
 	vFuture.ApprovedUpgrades = map[protocol.ConsensusVersion]uint64{}
 	Consensus[protocol.ConsensusFuture] = vFuture
 }
diff --git a/data/bookkeeping/block.go b/data/bookkeeping/block.go
index a5010ed20..54426066b 100644
--- a/data/bookkeeping/block.go
+++ b/data/bookkeeping/block.go
@@ -316,7 +316,7 @@ func (s UpgradeState) applyUpgradeVote(r basics.Round, vote UpgradeVote) (res Up
 
 		upgradeDelay := uint64(vote.UpgradeDelay)
 		if upgradeDelay > params.MaxUpgradeWaitRounds || upgradeDelay < params.MinUpgradeWaitRounds {
-			err = fmt.Errorf("applyUpgradeVote: proposed upgrade wait rounds %d out of permissible range", upgradeDelay)
+			err = fmt.Errorf("applyUpgradeVote: proposed upgrade wait rounds %d out of permissible range [%d, %d]", upgradeDelay, params.MinUpgradeWaitRounds, params.MaxUpgradeWaitRounds)
 			return
 		}
 
diff --git a/ledger/ledger_test.go b/ledger/ledger_test.go
index e06bb7400..6b2a0bbee 100644
--- a/ledger/ledger_test.go
+++ b/ledger/ledger_test.go
@@ -777,3 +777,83 @@ func TestLedgerSingleTxApplyDataV18(t *testing.T) {
 func TestLedgerSingleTxApplyDataFuture(t *testing.T) {
 	testLedgerSingleTxApplyData(t, protocol.ConsensusFuture)
 }
+
+func TestLedgerRegressionFaultyLeaseFirstValidCheckOld(t *testing.T) {
+	testLedgerRegressionFaultyLeaseFirstValidCheck2f3880f7(t, protocol.ConsensusV22)
+}
+
+func TestLedgerRegressionFaultyLeaseFirstValidCheckV23(t *testing.T) {
+	testLedgerRegressionFaultyLeaseFirstValidCheck2f3880f7(t, protocol.ConsensusV23)
+}
+
+func TestLedgerRegressionFaultyLeaseFirstValidCheck(t *testing.T) {
+	testLedgerRegressionFaultyLeaseFirstValidCheck2f3880f7(t, protocol.ConsensusCurrentVersion)
+}
+
+func TestLedgerRegressionFaultyLeaseFirstValidCheckFuture(t *testing.T) {
+	testLedgerRegressionFaultyLeaseFirstValidCheck2f3880f7(t, protocol.ConsensusFuture)
+}
+
+func testLedgerRegressionFaultyLeaseFirstValidCheck2f3880f7(t *testing.T, version protocol.ConsensusVersion) {
+	a := require.New(t)
+
+	backlogPool := execpool.MakeBacklog(nil, 0, execpool.LowPriority, nil)
+	defer backlogPool.Shutdown()
+
+	genesisInitState, initSecrets := testGenerateInitState(t, version)
+	const inMem = true
+	const archival = true
+	log := logging.TestingLog(t)
+	l, err := OpenLedger(log, t.Name(), inMem, genesisInitState, archival)
+	a.NoError(err, "could not open ledger")
+	defer l.Close()
+
+	proto := config.Consensus[version]
+	poolAddr := testPoolAddr
+	sinkAddr := testSinkAddr
+
+	initAccounts := genesisInitState.Accounts
+	var addrList []basics.Address
+	for addr := range initAccounts {
+		if addr != poolAddr && addr != sinkAddr {
+			addrList = append(addrList, addr)
+		}
+	}
+
+	correctTxHeader := transactions.Header{
+		Sender:      addrList[0],
+		Fee:         basics.MicroAlgos{Raw: proto.MinTxnFee * 2},
+		FirstValid:  l.Latest() + 1,
+		LastValid:   l.Latest() + 10,
+		GenesisID:   t.Name(),
+		GenesisHash: crypto.Hash([]byte(t.Name())),
+	}
+
+	correctPayFields := transactions.PaymentTxnFields{
+		Receiver: addrList[1],
+		Amount:   basics.MicroAlgos{Raw: initAccounts[addrList[0]].MicroAlgos.Raw / 10},
+	}
+
+	correctPay := transactions.Transaction{
+		Type:             protocol.PaymentTx,
+		Header:           correctTxHeader,
+		PaymentTxnFields: correctPayFields,
+	}
+
+	var ad transactions.ApplyData
+
+	correctPayLease := correctPay
+	correctPayLease.Sender = addrList[3]
+	correctPayLease.Lease[0] = 1
+
+	a.NoError(l.appendUnvalidatedTx(t, initAccounts, initSecrets, correctPayLease, ad), "could not add initial payment transaction")
+
+	correctPayLease.FirstValid = l.Latest() + 1
+	correctPayLease.LastValid = l.Latest() + 10
+
+	if proto.FixTransactionLeases {
+		a.Error(l.appendUnvalidatedTx(t, initAccounts, initSecrets, correctPayLease, ad), "added payment transaction with overlapping lease")
+	} else {
+		a.NoError(l.appendUnvalidatedTx(t, initAccounts, initSecrets, correctPayLease, ad), "should allow leasing payment transaction with newer FirstValid")
+	}
+}
diff --git a/ledger/txtail.go b/ledger/txtail.go
index bac5eb13c..f51fb64f4 100644
--- a/ledger/txtail.go
+++ b/ledger/txtail.go
@@ -36,7 +36,7 @@ type txTail struct {
 
 	lastValid map[basics.Round]map[transactions.Txid]struct{} // map tx.LastValid -> tx confirmed set
 
-	// duplicate detection queries with LastValid not before
+	// duplicate detection queries with LastValid before
 	// lowWaterMark are not guaranteed to succeed
 	lowWaterMark basics.Round // the last round known to be committed to disk
 }
@@ -128,7 +128,14 @@ func (t *txTail) isDup(proto config.ConsensusParams, current basics.Round, first
 	}
 
 	if proto.SupportTransactionLeases && (txl.lease != [32]byte{}) {
-		for rnd := firstValid; rnd <= lastValid; rnd++ {
+		firstChecked := firstValid
+		lastChecked := lastValid
+		if proto.FixTransactionLeases {
+			firstChecked = current.SubSaturate(basics.Round(proto.MaxTxnLife))
+			lastChecked = current
+		}
+
+		for rnd := firstChecked; rnd <= lastChecked; rnd++ {
 			expires, ok := t.recent[rnd].txleases[txl]
 			if ok && current <= expires {
 				return true, nil
diff --git a/protocol/consensus.go b/protocol/consensus.go
index 9fd872e56..86928db44 100644
--- a/protocol/consensus.go
+++ b/protocol/consensus.go
@@ -123,6 +123,11 @@ const ConsensusV22 = ConsensusVersion(
 	"https://github.com/algorandfoundation/specs/tree/57016b942f6d97e6d4c0688b373bb0a2fc85a1a2",
 )
 
+// ConsensusV23 fixes lease behavior.
+const ConsensusV23 = ConsensusVersion(
+	"https://github.com/algorandfoundation/specs/tree/e5f565421d720c6f75cdd186f7098495caf9101f",
+)
+
 // ConsensusFuture is a protocol that should not appear in any production
 // network, but is used to test features before they are released.
 const ConsensusFuture = ConsensusVersion(
@@ -135,7 +140,7 @@ const ConsensusFuture = ConsensusVersion(
 
 // ConsensusCurrentVersion is the latest version and should be used
 // when a specific version is not provided.
-const ConsensusCurrentVersion = ConsensusV22
+const ConsensusCurrentVersion = ConsensusV23
 
 // Error is used to indicate that an unsupported protocol has been detected.
 type Error ConsensusVersion
diff --git a/test/e2e-go/features/catchup/basicCatchup_test.go b/test/e2e-go/features/catchup/basicCatchup_test.go
index c2ad4df65..5e7c15b8e 100644
--- a/test/e2e-go/features/catchup/basicCatchup_test.go
+++ b/test/e2e-go/features/catchup/basicCatchup_test.go
@@ -213,6 +213,7 @@ func TestStoppedCatchupOnUnsupported(t *testing.T) {
 	testUnupgradedProtocol.UpgradeVoteRounds = 3
 	testUnupgradedProtocol.UpgradeThreshold = 2
 	testUnupgradedProtocol.DefaultUpgradeWaitRounds = 3
+	testUnupgradedProtocol.MinUpgradeWaitRounds = 0
 
 	testUnupgradedProtocol.ApprovedUpgrades[consensusTestUnupgradedToProtocol] = 0
 	consensus[consensusTestUnupgradedProtocol] = testUnupgradedProtocol
diff --git a/test/e2e-go/features/transactions/lease_test.go b/test/e2e-go/features/transactions/lease_test.go
index cd2d0698e..29366f768 100644
--- a/test/e2e-go/features/transactions/lease_test.go
+++ b/test/e2e-go/features/transactions/lease_test.go
@@ -31,7 +31,7 @@ func TestLeaseTransactionsSameSender(t *testing.T) {
 	a := require.New(t)
 
 	var fixture fixtures.RestClientFixture
-	fixture.Setup(t, filepath.Join("nettemplates", "TwoNodes50EachFuture.json"))
+	fixture.Setup(t, filepath.Join("nettemplates", "TwoNodes50Each.json"))
 	defer fixture.Shutdown()
 
 	client := fixture.LibGoalClient
@@ -85,12 +85,143 @@ func TestLeaseTransactionsSameSender(t *testing.T) {
 	a.Equal(bal2, uint64(0))
 }
 
+func TestLeaseRegressionFaultyFirstValidCheckOld_2f3880f7(t *testing.T) {
+	t.Parallel()
+	a := require.New(t)
+
+	var fixture fixtures.RestClientFixture
+	fixture.Setup(t, filepath.Join("nettemplates", "TwoNodes50EachV22.json"))
+	defer fixture.Shutdown()
+
+	client := fixture.LibGoalClient
+	accountList, err := fixture.GetWalletsSortedByBalance()
+	a.NoError(err)
+	account0 := accountList[0].Address
+	wh, err := client.GetUnencryptedWalletHandle()
+	a.NoError(err)
+
+	account1, err := client.GenerateAddress(wh)
+	a.NoError(err)
+
+	account2, err := client.GenerateAddress(wh)
+	a.NoError(err)
+
+	lease := [32]byte{1, 2, 3, 4}
+
+	// construct transactions for sending money to account1 and account2
+	// from same sender with identical lease
+	tx1, err := client.ConstructPayment(account0, account1, 0, 1000000, nil, "", lease, 0, 0)
+	a.NoError(err)
+
+	stx1, err := client.SignTransactionWithWallet(wh, nil, tx1)
+	a.NoError(err)
+
+	// submitting the first transaction should succeed
+	_, err = client.BroadcastTransaction(stx1)
+	a.NoError(err)
+
+	// wait for the txids and check balance
+	txids := make(map[string]string)
+	txids[stx1.Txn.ID().String()] = account0
+
+	_, curRound := fixture.GetBalanceAndRound(account0)
+	confirmed := fixture.WaitForAllTxnsToConfirm(curRound+5, txids)
+	a.True(confirmed, "lease txn confirmed")
+
+	bal1, _ := fixture.GetBalanceAndRound(account1)
+	bal2, _ := fixture.GetBalanceAndRound(account2)
+	a.Equal(bal1, uint64(1000000))
+	a.Equal(bal2, uint64(0))
+
+	tx2, err := client.ConstructPayment(account0, account2, 0, 2000000, nil, "", lease, 0, 0)
+	a.NoError(err)
+
+	stx2, err := client.SignTransactionWithWallet(wh, nil, tx2)
+	a.NoError(err)
+
+	// submitting the second transaction should succeed
+	_, err = client.BroadcastTransaction(stx2)
+	a.NoError(err)
+
+	// wait for the txids and check balance
+	txids = make(map[string]string)
+	txids[stx2.Txn.ID().String()] = account0
+
+	_, curRound = fixture.GetBalanceAndRound(account0)
+	confirmed = fixture.WaitForAllTxnsToConfirm(curRound+5, txids)
+	a.True(confirmed, "lease txn confirmed")
+
+	bal1, _ = fixture.GetBalanceAndRound(account1)
+	bal2, _ = fixture.GetBalanceAndRound(account2)
+	a.Equal(bal1, uint64(1000000))
+	a.Equal(bal2, uint64(2000000))
+}
+
+func TestLeaseRegressionFaultyFirstValidCheckNew_2f3880f7(t *testing.T) {
+	t.Parallel()
+	a := require.New(t)
+
+	var fixture fixtures.RestClientFixture
+	fixture.Setup(t, filepath.Join("nettemplates", "TwoNodes50Each.json"))
+	defer fixture.Shutdown()
+
+	client := fixture.LibGoalClient
+	accountList, err := fixture.GetWalletsSortedByBalance()
+	a.NoError(err)
+	account0 := accountList[0].Address
+	wh, err := client.GetUnencryptedWalletHandle()
+	a.NoError(err)
+
+	account1, err := client.GenerateAddress(wh)
+	a.NoError(err)
+
+	account2, err := client.GenerateAddress(wh)
+	a.NoError(err)
+
+	lease := [32]byte{1, 2, 3, 4}
+
+	// construct transactions for sending money to account1 and account2
+	// from same sender with identical lease
+	tx1, err := client.ConstructPayment(account0, account1, 0, 1000000, nil, "", lease, 0, 0)
+	a.NoError(err)
+
+	stx1, err := client.SignTransactionWithWallet(wh, nil, tx1)
+	a.NoError(err)
+
+	// submitting the first transaction should succeed
+	_, err = client.BroadcastTransaction(stx1)
+	a.NoError(err)
+
+	// wait for the txids and check balance
+	txids := make(map[string]string)
+	txids[stx1.Txn.ID().String()] = account0
+
+	_, curRound := fixture.GetBalanceAndRound(account0)
+	confirmed := fixture.WaitForAllTxnsToConfirm(curRound+5, txids)
+	a.True(confirmed, "lease txn confirmed")
+
+	bal1, _ := fixture.GetBalanceAndRound(account1)
+	bal2, _ := fixture.GetBalanceAndRound(account2)
+	a.Equal(bal1, uint64(1000000))
+	a.Equal(bal2, uint64(0))
+
+	tx2, err := client.ConstructPayment(account0, account2, 0, 2000000, nil, "", lease, 0, 0)
+	a.NoError(err)
+
+	stx2, err := client.SignTransactionWithWallet(wh, nil, tx2)
+	a.NoError(err)
+
+	// submitting the second transaction should fail
+	_, err = client.BroadcastTransaction(stx2)
+	a.Error(err)
+}
+
 func TestLeaseTransactionsSameSenderDifferentLease(t *testing.T) {
 	t.Parallel()
 	a := require.New(t)
 
 	var fixture fixtures.RestClientFixture
-	fixture.Setup(t, filepath.Join("nettemplates", "TwoNodes50EachFuture.json"))
+	fixture.Setup(t, filepath.Join("nettemplates", "TwoNodes50Each.json"))
 	defer fixture.Shutdown()
 
 	client := fixture.LibGoalClient
@@ -151,7 +282,7 @@ func TestLeaseTransactionsDifferentSender(t *testing.T) {
 	a := require.New(t)
 
 	var fixture fixtures.RestClientFixture
-	fixture.Setup(t, filepath.Join("nettemplates", "TwoNodes50EachFuture.json"))
+	fixture.Setup(t, filepath.Join("nettemplates", "TwoNodes50Each.json"))
 	defer fixture.Shutdown()
 
 	client := fixture.LibGoalClient
@@ -225,7 +356,7 @@ func TestOverlappingLeases(t *testing.T) {
 	a := require.New(t)
 
 	var fixture fixtures.RestClientFixture
-	fixture.Setup(t, filepath.Join("nettemplates", "TwoNodes50EachFuture.json"))
+	fixture.Setup(t, filepath.Join("nettemplates", "TwoNodes50Each.json"))
 	defer fixture.Shutdown()
 
 	client := fixture.LibGoalClient
diff --git a/test/e2e-go/upgrades/send_receive_upgrade_test.go b/test/e2e-go/upgrades/send_receive_upgrade_test.go
index e748a5518..3066db8e4 100644
--- a/test/e2e-go/upgrades/send_receive_upgrade_test.go
+++ b/test/e2e-go/upgrades/send_receive_upgrade_test.go
@@ -104,6 +104,20 @@ func TestAccountsCanSendMoneyAcrossUpgradeV15toV16(t *testing.T) {
 	testAccountsCanSendMoneyAcrossUpgrade(t, filepath.Join("nettemplates", "TwoNodes50EachV15Upgrade.json"))
 }
 
+func TestAccountsCanSendMoneyAcrossUpgradeV21toV22(t *testing.T) {
+	if runtime.GOOS == "darwin" {
+		t.Skip()
+	}
+	testAccountsCanSendMoneyAcrossUpgrade(t, filepath.Join("nettemplates", "TwoNodes50EachV21Upgrade.json"))
+}
+
+func TestAccountsCanSendMoneyAcrossUpgradeV22toV23(t *testing.T) {
+	if runtime.GOOS == "darwin" {
+		t.Skip()
+	}
+	testAccountsCanSendMoneyAcrossUpgrade(t, filepath.Join("nettemplates", "TwoNodes50EachV22Upgrade.json"))
+}
+
 // ConsensusTestFastUpgrade is meant for testing of protocol upgrades:
 // during testing, it is equivalent to another protocol with the exception
 // of the upgrade parameters, which allow for upgrades to take place after
@@ -120,6 +134,8 @@ func generateFastUpgradeConsensus() (fastUpgradeProtocols config.ConsensusProtoc
 		fastParams.UpgradeVoteRounds = 5
 		fastParams.UpgradeThreshold = 3
 		fastParams.DefaultUpgradeWaitRounds = 5
+		fastParams.MinUpgradeWaitRounds = 0
+		fastParams.MaxUpgradeWaitRounds = 0
 		fastParams.MaxVersionStringLen += len(consensusTestFastUpgrade(""))
 		fastParams.ApprovedUpgrades = make(map[protocol.ConsensusVersion]uint64)
 
diff --git a/test/testdata/nettemplates/TwoNodes50EachV21Upgrade.json b/test/testdata/nettemplates/TwoNodes50EachV21Upgrade.json
new file mode 100644
index 000000000..d5729d5b5
--- /dev/null
+++ b/test/testdata/nettemplates/TwoNodes50EachV21Upgrade.json
@@ -0,0 +1,35 @@
+{
+    "Genesis": {
+        "NetworkName": "tbd",
+        "ConsensusProtocol": "test-fast-upgrade-https://github.com/algorandfoundation/specs/tree/8096e2df2da75c3339986317f9abe69d4fa86b4b",
+        "Wallets": [
+            {
+                "Name": "Wallet1",
+                "Stake": 50,
+                "Online": true
+            },
+            {
+                "Name": "Wallet2",
+                "Stake": 50,
+                "Online": true
+            }
+        ]
+    },
+    "Nodes": [
+        {
+            "Name": "Primary",
+            "IsRelay": true,
+            "Wallets": [
+                { "Name": "Wallet1",
+                  "ParticipationOnly": false }
+            ]
+        },
+        {
+            "Name": "Node",
+            "Wallets": [
+                { "Name": "Wallet2",
+                  "ParticipationOnly": false }
+            ]
+        }
+    ]
+}
diff --git a/test/testdata/nettemplates/TwoNodes50EachV22.json b/test/testdata/nettemplates/TwoNodes50EachV22.json
new file mode 100644
index 000000000..4e9dddcbb
--- /dev/null
+++ b/test/testdata/nettemplates/TwoNodes50EachV22.json
@@ -0,0 +1,35 @@
+{
+    "Genesis": {
+        "NetworkName": "tbd",
+        "ConsensusProtocol": "https://github.com/algorandfoundation/specs/tree/57016b942f6d97e6d4c0688b373bb0a2fc85a1a2",
+        "Wallets": [
+            {
+                "Name": "Wallet1",
+                "Stake": 50,
+                "Online": true
+            },
+            {
+                "Name": "Wallet2",
+                "Stake": 50,
+                "Online": true
+            }
+        ]
+    },
+    "Nodes": [
+        {
+            "Name": "Primary",
+            "IsRelay": true,
+            "Wallets": [
+                { "Name": "Wallet1",
+                  "ParticipationOnly": false }
+            ]
+        },
+        {
+            "Name": "Node",
+            "Wallets": [
+                { "Name": "Wallet2",
+                  "ParticipationOnly": false }
+            ]
+        }
+    ]
+}
diff --git a/test/testdata/nettemplates/TwoNodes50EachV22Upgrade.json b/test/testdata/nettemplates/TwoNodes50EachV22Upgrade.json
new file mode 100644
index 000000000..f702f491f
--- /dev/null
+++ b/test/testdata/nettemplates/TwoNodes50EachV22Upgrade.json
@@ -0,0 +1,35 @@
+{
+    "Genesis": {
+        "NetworkName": "tbd",
+        "ConsensusProtocol": "test-fast-upgrade-https://github.com/algorandfoundation/specs/tree/57016b942f6d97e6d4c0688b373bb0a2fc85a1a2",
+        "Wallets": [
+            {
+                "Name": "Wallet1",
+                "Stake": 50,
+                "Online": true
+            },
+            {
+                "Name": "Wallet2",
+                "Stake": 50,
+                "Online": true
+            }
+        ]
+    },
+    "Nodes": [
+        {
+            "Name": "Primary",
+            "IsRelay": true,
+            "Wallets": [
+                { "Name": "Wallet1",
+                  "ParticipationOnly": false }
+            ]
+        },
+        {
+            "Name": "Node",
+            "Wallets": [
+                { "Name": "Wallet2",
+                  "ParticipationOnly": false }
+            ]
+        }
+    ]
+}