libgoal: set FirstValid to LastRound to prevent early tnxs (#5622)

libgoal/libgoal.go

@@ -528,7 +528,17 @@ func computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxn
 	}
 
 	if firstValid == 0 {
-		firstValid = lastRound + 1
+		// current node might be a bit ahead of the network, and to prevent sibling nodes from rejecting the transaction
+		// because it's FirstValid is greater than their pending block evaluator.
+		// For example, a node just added block 100 and immediately sending a new transaction.
+		// The other side is lagging behind by 100ms and its LastRound is 99 so its transaction pools accepts txns for rounds 100+.
+		// This means the node client have to set FirstValid to 100 or below.
+		if lastRound > 0 {
+			firstValid = lastRound
+		} else {
+			// there is no practical sense to set FirstValid to 0, so we set it to 1
+			firstValid = 1
+		}
 	}
 
 	if validRounds != 0 {

libgoal/libgoal_test.go

@@ -38,42 +38,42 @@ func TestValidRounds(t *testing.T) {
 	validRounds = 0
 	fv, lv, err := computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
 	a.NoError(err)
-	a.Equal(lastRound+1, fv)
+	a.Equal(lastRound, fv)
 	a.Equal(fv+maxTxnLife, lv)
 
 	firstValid = 0
 	lastValid = 0
 	validRounds = maxTxnLife + 1
 	fv, lv, err = computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
 	a.NoError(err)
-	a.Equal(lastRound+1, fv)
+	a.Equal(lastRound, fv)
 	a.Equal(fv+maxTxnLife, lv)
 
 	firstValid = 0
 	lastValid = 0
 	validRounds = maxTxnLife + 2
-	fv, lv, err = computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
+	_, _, err = computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
 	a.Error(err)
 	a.Equal("cannot construct transaction: txn validity period 1001 is greater than protocol max txn lifetime 1000", err.Error())
 
 	firstValid = 0
 	lastValid = 1
 	validRounds = 2
-	fv, lv, err = computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
+	_, _, err = computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
 	a.Error(err)
 	a.Equal("cannot construct transaction: ambiguous input: lastValid = 1, validRounds = 2", err.Error())
 
 	firstValid = 2
 	lastValid = 1
 	validRounds = 0
-	fv, lv, err = computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
+	_, _, err = computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
 	a.Error(err)
 	a.Equal("cannot construct transaction: txn would first be valid on round 2 which is after last valid round 1", err.Error())
 
 	firstValid = 1
 	lastValid = maxTxnLife + 2
 	validRounds = 0
-	fv, lv, err = computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
+	_, _, err = computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
 	a.Error(err)
 	a.Equal("cannot construct transaction: txn validity period ( 1 to 1002 ) is greater than protocol max txn lifetime 1000", err.Error())
 
@@ -90,24 +90,24 @@ func TestValidRounds(t *testing.T) {
 	validRounds = 0
 	fv, lv, err = computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
 	a.NoError(err)
-	a.Equal(lastRound+1, fv)
+	a.Equal(lastRound, fv)
 	a.Equal(lastRound+1, lv)
 
 	firstValid = 0
 	lastValid = 0
 	validRounds = 1
 	fv, lv, err = computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
 	a.NoError(err)
-	a.Equal(lastRound+1, fv)
-	a.Equal(lastRound+1, lv)
+	a.Equal(lastRound, fv)
+	a.Equal(lastRound, lv)
 
 	firstValid = 0
 	lastValid = 0
 	validRounds = maxTxnLife
 	fv, lv, err = computeValidityRounds(firstValid, lastValid, validRounds, lastRound, maxTxnLife)
 	a.NoError(err)
-	a.Equal(lastRound+1, fv)
-	a.Equal(lastRound+maxTxnLife, lv)
+	a.Equal(lastRound, fv)
+	a.Equal(lastRound+maxTxnLife-1, lv)
 
 	firstValid = 1
 	lastValid = 0

test/e2e-go/features/transactions/asset_test.go

@@ -80,7 +80,7 @@ func TestAssetValidRounds(t *testing.T) {
 	validRounds = cparams.MaxTxnLife + 1
 	firstValid, lastValid, lastRound, err = client.ComputeValidityRounds(firstValid, lastValid, validRounds)
 	a.NoError(err)
-	a.Equal(lastRound+1, firstValid)
+	a.True(firstValid == 1 || firstValid == lastRound)
 	a.Equal(firstValid+cparams.MaxTxnLife, lastValid)
 
 	firstValid = 0

test/e2e-go/features/transactions/lease_test.go

@@ -136,11 +136,11 @@ func TestLeaseRegressionFaultyFirstValidCheckOld_2f3880f7(t *testing.T) {
 	a.True(confirmed, "lease txn confirmed")
 
 	bal1, _ := fixture.GetBalanceAndRound(account1)
-	bal2, _ := fixture.GetBalanceAndRound(account2)
+	bal2, curRound := 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)
+	tx2, err := client.ConstructPayment(account0, account2, 0, 2000000, nil, "", lease, basics.Round(curRound)+1, 0)
 	a.NoError(err)
 
 	stx2, err := client.SignTransactionWithWallet(wh, nil, tx2)

test/scripts/e2e_subs/e2e-teal.sh

@@ -71,7 +71,8 @@ while [ $CROUND -lt $TIMEOUT_ROUND ]; do
     CROUND=$(goal node status | grep 'Last committed block:'|awk '{ print $4 }')
 done
 
-${gcmd} clerk send --from-program ${TEMPDIR}/tlhc.teal --to ${ACCOUNT} --close-to ${ACCOUNT} --amount 1 --argb64 AA==
+# send txn that valid right after the TIMEOUT_ROUND
+${gcmd} clerk send --firstvalid $((${TIMEOUT_ROUND} + 1))  --from-program ${TEMPDIR}/tlhc.teal --to ${ACCOUNT} --close-to ${ACCOUNT} --amount 1 --argb64 AA==
 
 cat >${TEMPDIR}/true.teal<<EOF
 #pragma version 2

test/scripts/e2e_subs/limit-swap-test.sh

@@ -73,7 +73,7 @@ while [ $ROUND -lt $TIMEOUT_ROUND ]; do
 done
 
 echo "recover asset"
-${gcmd} asset send --assetid ${ASSET_ID} -t ${ZERO_ADDRESS} -a 0 -c ${ACCOUNT} -f ${ACCOUNT_ASSET_TRADER} -o ${TEMPDIR}/bclose.tx
+${gcmd} asset send --firstvalid $((${TIMEOUT_ROUND} +  1)) --assetid ${ASSET_ID} -t ${ZERO_ADDRESS} -a 0 -c ${ACCOUNT} -f ${ACCOUNT_ASSET_TRADER} -o ${TEMPDIR}/bclose.tx
 
 ${gcmd} clerk sign -i ${TEMPDIR}/bclose.tx -p ${TEMPDIR}/limit-order-b.teal -o ${TEMPDIR}/bclose.stx