Merge pull request #575 from SiaFoundation/nate/fix-rejections-for-pa…

…rtial-parents

Retry formation transaction sets

.changeset/fixed_an_issue_with_contracts_sometimes_being_rejected_if_one_of_their_parent_transactions_was_confirmed_in_an_earlier_block_.md

@@ -0,0 +1,5 @@
+---
+default: patch
+---
+
+# Fixed an issue with contracts sometimes being rejected if one of their parent transactions was confirmed in an earlier block 

go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/mattn/go-sqlite3 v1.14.24
 	github.com/shopspring/decimal v1.4.0
 	go.sia.tech/core v0.9.0
-	go.sia.tech/coreutils v0.9.1
+	go.sia.tech/coreutils v0.10.0
 	go.sia.tech/jape v0.12.1
 	go.sia.tech/mux v1.3.0
 	go.sia.tech/web/hostd v0.56.0

go.sum

@@ -42,8 +42,8 @@ go.etcd.io/bbolt v1.3.11 h1:yGEzV1wPz2yVCLsD8ZAiGHhHVlczyC9d1rP43/VCRJ0=
 go.etcd.io/bbolt v1.3.11/go.mod h1:dksAq7YMXoljX0xu6VF5DMZGbhYYoLUalEiSySYAS4I=
 go.sia.tech/core v0.9.0 h1:qV7V8nkNaPvBEhkbwgrETTkb7JCMcAnKUQt9nUumP4k=
 go.sia.tech/core v0.9.0/go.mod h1:3NAvYHuzAZg9vP6pyIMOxjTkgHBQ3vx9cXTqRF6oEa4=
-go.sia.tech/coreutils v0.9.1 h1:2SukWrF9o18HIG+BNmNk4SZw48+h+cM0gR8jMtG2cQ4=
-go.sia.tech/coreutils v0.9.1/go.mod h1:A/tNYSBdryxCFaIpvW04YvdQ4e2j8iGuHoIw70+ZYXc=
+go.sia.tech/coreutils v0.10.0 h1:uKkxq2llz49vDRUDdGlPbHCMKEf5OJhCDkGxIZiLedA=
+go.sia.tech/coreutils v0.10.0/go.mod h1:/m6/PFV377MeJ42uO1xd+4//B1TNaQYF7DURrILHDvA=
 go.sia.tech/jape v0.12.1 h1:xr+o9V8FO8ScRqbSaqYf9bjj1UJ2eipZuNcI1nYousU=
 go.sia.tech/jape v0.12.1/go.mod h1:wU+h6Wh5olDjkPXjF0tbZ1GDgoZ6VTi4naFw91yyWC4=
 go.sia.tech/mux v1.3.0 h1:hgR34IEkqvfBKUJkAzGi31OADeW2y7D6Bmy/Jcbop9c=

host/contracts/manager_test.go

@@ -802,6 +802,117 @@ func TestContractLifecycle(t *testing.T) {
 		assertContractStatus(t, node.Contracts, rev.Revision.ParentID, contracts.ContractStatusPending)
 		assertContractMetrics(t, node.Store, 0, 0, types.ZeroCurrency, types.ZeroCurrency)
 	})
+
+	t.Run("partially confirmed formation set", func(t *testing.T) {
+		hostKey, renterKey := types.GeneratePrivateKey(), types.GeneratePrivateKey()
+
+		dir := t.TempDir()
+		log := zaptest.NewLogger(t)
+
+		network, genesis := testutil.V1Network()
+		node := testutil.NewHostNode(t, hostKey, network, genesis, log)
+
+		result := make(chan error, 1)
+		if _, err := node.Volumes.AddVolume(context.Background(), filepath.Join(dir, "data.dat"), 10, result); err != nil {
+			t.Fatal(err)
+		} else if err := <-result; err != nil {
+			t.Fatal(err)
+		}
+
+		testutil.MineAndSync(t, node, node.Wallet.Address(), int(network.MaturityDelay+5))
+
+		renterFunds := types.Siacoins(500)
+		hostCollateral := types.Siacoins(1000)
+
+		contract := rhp2.PrepareContractFormation(renterKey.PublicKey(), hostKey.PublicKey(), renterFunds, hostCollateral, node.Chain.Tip().Height+10, rhp2.HostSettings{WindowSize: 10}, node.Wallet.Address())
+		state := node.Chain.TipState()
+		formationCost := rhp2.ContractFormationCost(state, contract, types.ZeroCurrency)
+		// create a thread of multiple ephemeral outputs
+
+		contractUnlockConditions := types.UnlockConditions{
+			PublicKeys: []types.UnlockKey{
+				renterKey.PublicKey().UnlockKey(),
+				hostKey.PublicKey().UnlockKey(),
+			},
+			SignaturesRequired: 2,
+		}
+		formationSet := []types.Transaction{
+			{
+				ArbitraryData: [][]byte{[]byte("setup txn 1")},
+				SiacoinOutputs: []types.SiacoinOutput{
+					{Address: node.Wallet.Address(), Value: formationCost.Add(hostCollateral)},
+				},
+			},
+			{
+				ArbitraryData: [][]byte{[]byte("setup txn 2")},
+				SiacoinOutputs: []types.SiacoinOutput{
+					{Address: node.Wallet.Address(), Value: formationCost.Add(hostCollateral)},
+				},
+			},
+			{
+				FileContracts: []types.FileContract{contract},
+			},
+		}
+		// fund the formation transaction
+		toSign, err := node.Wallet.FundTransaction(&formationSet[0], formationCost.Add(hostCollateral), true)
+		if err != nil {
+			t.Fatal("failed to fund transaction:", err)
+		}
+		node.Wallet.SignTransaction(&formationSet[0], toSign, types.CoveredFields{WholeTransaction: true})
+
+		// add and sign the ephemeral inputs
+		formationSet[1].SiacoinInputs = []types.SiacoinInput{
+			{
+				ParentID:         formationSet[0].SiacoinOutputID(0),
+				UnlockConditions: types.StandardUnlockConditions(hostKey.PublicKey()),
+			},
+		}
+		node.Wallet.SignTransaction(&formationSet[1], []types.Hash256{types.Hash256(formationSet[0].SiacoinOutputID(0))}, types.CoveredFields{WholeTransaction: true})
+
+		formationSet[2].SiacoinInputs = []types.SiacoinInput{
+			{
+				ParentID:         formationSet[1].SiacoinOutputID(0),
+				UnlockConditions: types.StandardUnlockConditions(hostKey.PublicKey()),
+			},
+		}
+		node.Wallet.SignTransaction(&formationSet[2], []types.Hash256{types.Hash256(formationSet[1].SiacoinOutputID(0))}, types.CoveredFields{WholeTransaction: true})
+
+		revision := types.FileContractRevision{
+			ParentID:         formationSet[2].FileContractID(0),
+			UnlockConditions: contractUnlockConditions,
+			FileContract:     formationSet[2].FileContracts[0],
+		}
+
+		// broadcast the first transaction only
+		if _, err := node.Chain.AddPoolTransactions([]types.Transaction{formationSet[0]}); err != nil {
+			t.Fatal(err)
+		}
+
+		revision.RevisionNumber = 1
+		sigHash := hashRevision(revision)
+		rev := contracts.SignedRevision{
+			Revision:        revision,
+			HostSignature:   hostKey.SignHash(sigHash),
+			RenterSignature: renterKey.SignHash(sigHash),
+		}
+		if err := node.Contracts.AddContract(rev, formationSet, hostCollateral, contracts.Usage{}); err != nil {
+			t.Fatal(err)
+		}
+
+		assertContractStatus(t, node.Contracts, rev.Revision.ParentID, contracts.ContractStatusPending)
+		// pending contracts do not contribute to metrics
+		assertContractMetrics(t, node.Store, 0, 0, types.ZeroCurrency, types.ZeroCurrency)
+
+		// mine to rebroadcast the formation set
+		testutil.MineAndSync(t, node, types.VoidAddress, 1)
+		assertContractStatus(t, node.Contracts, rev.Revision.ParentID, contracts.ContractStatusPending)
+		assertContractMetrics(t, node.Store, 0, 0, types.ZeroCurrency, types.ZeroCurrency)
+
+		// mine to confirm the contract
+		testutil.MineAndSync(t, node, types.VoidAddress, 1)
+		assertContractStatus(t, node.Contracts, rev.Revision.ParentID, contracts.ContractStatusActive)
+		assertContractMetrics(t, node.Store, 1, 0, hostCollateral, types.ZeroCurrency)
+	})
 }
 
 func TestV2ContractLifecycle(t *testing.T) {

host/contracts/update.go

@@ -1,6 +1,7 @@
 package contracts
 
 import (
+	"errors"
 	"fmt"
 
 	"go.sia.tech/core/consensus"
@@ -196,13 +197,15 @@ func (cm *Manager) ProcessActions(index types.ChainIndex) error {
 			log.Debug("skipping formation set missing file contract")
 			continue
 		}
-		if _, err := cm.chain.AddPoolTransactions(formationSet); err != nil {
+		formationTxn := formationSet[len(formationSet)-1]
+		contractID := formationSet[len(formationSet)-1].FileContractID(0)
+		log := log.With(zap.Stringer("contractID", contractID), zap.Stringer("transactionID", formationTxn.ID()))
+		formationSet, err := tryFormationBroadcast(cm.chain, formationSet)
+		if err != nil {
 			log.Error("failed to add formation transaction to pool", zap.Error(err))
-			continue
 		}
 		cm.syncer.BroadcastTransactionSet(formationSet)
-		contractID := formationSet[len(formationSet)-1].FileContractID(0)
-		log.Debug("rebroadcast formation transaction", zap.Stringer("contractID", contractID), zap.String("transactionID", formationSet[len(formationSet)-1].ID().String()))
+		log.Debug("rebroadcast formation transaction", zap.String("transactionID", formationTxn.ID().String()))
 	}
 
 	for _, revision := range actions.BroadcastRevision {
@@ -597,3 +600,29 @@ func (cm *Manager) UpdateChainState(tx UpdateStateTx, reverted []chain.RevertUpd
 	}
 	return nil
 }
+
+// tryFormationBroadcast is a helper function that attempts to broadcast a formation
+// transaction set. Due to the nature of the transaction pool, it is possible
+// a transaction will not be accepted if one of the parent transactions has
+// already been confirmed. This function will retry all of the transactions in the set
+// sequentially to attempt to remove any transactions that may have already been confirmed.
+func tryFormationBroadcast(cm ChainManager, txnset []types.Transaction) ([]types.Transaction, error) {
+	if len(txnset) == 0 {
+		return nil, nil
+	} else if len(txnset[len(txnset)-1].FileContracts) == 0 {
+		return nil, errors.New("missing file contract")
+	}
+	formationTxn := txnset[len(txnset)-1]
+	_, err := cm.AddPoolTransactions(txnset)
+	if err == nil {
+		return txnset, nil
+	}
+
+	for _, txn := range txnset {
+		cm.AddPoolTransactions(append(cm.UnconfirmedParents(txn), txn)) // error is ignored because it will be caught below
+	}
+
+	txnset = append(cm.UnconfirmedParents(formationTxn), formationTxn)
+	_, err = cm.AddPoolTransactions(txnset)
+	return txnset, err
+}