From e6a04eabe47800e02c20b5a64231b2820667da28 Mon Sep 17 00:00:00 2001 From: Chris Schinnerl <3903476+ChrisSchinnerl@users.noreply.github.com> Date: Wed, 17 Jun 2026 10:31:26 +0200 Subject: [PATCH 1/6] add RemoveV2PoolTransactions --- chain/manager.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ rhp/v4/server.go | 16 +++++++++++++--- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/chain/manager.go b/chain/manager.go index 8b053162..ccc5673a 100644 --- a/chain/manager.go +++ b/chain/manager.go @@ -1443,6 +1443,50 @@ func (m *Manager) AddV2PoolTransactions(basis types.ChainIndex, txns []types.V2T return false, nil } +// RemoveV2PoolTransactions removes the given transactions from the transaction +// pool, along with any pooled transactions that depend on them. It is intended +// for abandoning a locally-added transaction set that will not be broadcast. +// Transactions that are not in the pool are ignored. +func (m *Manager) RemoveV2PoolTransactions(ids []types.TransactionID) { + m.mu.Lock() + defer m.mu.Unlock() + + remove := make(map[types.TransactionID]bool, len(ids)) + for _, id := range ids { + remove[id] = true + } + + filtered := m.txpool.v2txns[:0] + var removed bool + for _, txn := range m.txpool.v2txns { + if remove[txn.ID()] { + removed = true + continue + } + filtered = append(filtered, txn) + } + if !removed { + return + } + m.txpool.v2txns = filtered + + // force a full revalidation + m.txpool.ms = nil + m.txpool.medianFee = nil + m.revalidatePool() + + // release lock while notifying listeners + fns := make([]func(), 0, len(m.onPool)) + for _, fn := range m.onPool { + fns = append(fns, fn) + } + m.mu.Unlock() + for _, fn := range fns { + fn() + } + m.mu.Lock() +} + // NewManager returns a Manager initialized with the provided Store and State. func NewManager(store Store, cs consensus.State, opts ...ManagerOption) *Manager { m := &Manager{ diff --git a/rhp/v4/server.go b/rhp/v4/server.go index bd06bbc3..454a9b12 100644 --- a/rhp/v4/server.go +++ b/rhp/v4/server.go @@ -42,6 +42,10 @@ type ( // AddV2PoolTransactions validates a transaction set and adds it to the // transaction pool. AddV2PoolTransactions(types.ChainIndex, []types.V2Transaction) (known bool, err error) + // RemoveV2PoolTransactions removes the given transactions from the + // transaction pool, along with any pooled transactions that depend on + // them. Transactions that are not in the pool are ignored. + RemoveV2PoolTransactions([]types.TransactionID) // RecommendedFee returns the recommended fee per weight RecommendedFee() types.Currency @@ -787,8 +791,10 @@ func (s *Server) handleRPCFormContract(stream net.Conn) error { if broadcast { return } - // release the inputs if the transaction is not going to be broadcast + // release the inputs and remove the transaction from the pool if it is + // not going to be broadcast s.wallet.ReleaseInputs(nil, []types.V2Transaction{formationTxn}) + s.chain.RemoveV2PoolTransactions([]types.TransactionID{formationTxn.ID()}) }() // sign the transaction inputs s.wallet.SignV2Inputs(&formationTxn, toSign) @@ -953,8 +959,10 @@ func (s *Server) handleRPCRefreshContract(stream net.Conn, partial bool) error { if broadcast { return } - // release the locked UTXOs if the transaction is not going to be broadcast + // release the inputs and remove the transaction from the pool if it is + // not going to be broadcast s.wallet.ReleaseInputs(nil, []types.V2Transaction{renewalTxn}) + s.chain.RemoveV2PoolTransactions([]types.TransactionID{renewalTxn.ID()}) }() // update renter inputs to reflect our chain state @@ -1134,8 +1142,10 @@ func (s *Server) handleRPCRenewContract(stream net.Conn) error { if broadcast { return } - // release the locked UTXOs if the transaction is not going to be broadcast + // release the inputs and remove the transaction from the pool if it is + // not going to be broadcast s.wallet.ReleaseInputs(nil, []types.V2Transaction{renewalTxn}) + s.chain.RemoveV2PoolTransactions([]types.TransactionID{renewalTxn.ID()}) }() // update renter inputs to reflect our chain state From cab67ffca59bab271f0b5b074e6007aeb329fdb8 Mon Sep 17 00:00:00 2001 From: Chris Schinnerl <3903476+ChrisSchinnerl@users.noreply.github.com> Date: Wed, 17 Jun 2026 10:37:03 +0200 Subject: [PATCH 2/6] add TestRemoveV2PoolTransactions --- chain/pool_test.go | 131 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) diff --git a/chain/pool_test.go b/chain/pool_test.go index 143ad17c..3d9b939a 100644 --- a/chain/pool_test.go +++ b/chain/pool_test.go @@ -107,3 +107,134 @@ func TestAddV2PoolTransactionsRecover(t *testing.T) { t.Fatalf("expected invalid transaction, got %v", err) } } + +func TestRemoveV2PoolTransactions(t *testing.T) { + // setup returns a fresh manager along with a spendable siacoin element and + // the key/policy needed to spend it. + setup := func(t *testing.T) (*chain.Manager, types.ChainIndex, types.SiacoinElement, types.PrivateKey, types.SpendPolicy) { + n, genesisBlock := testutil.V2Network() + sk := types.GeneratePrivateKey() + sp := types.PolicyPublicKey(sk.PublicKey()) + addr := sp.Address() + + store, genesisState, err := chain.NewDBStore(chain.NewMemDB(), n, genesisBlock, nil) + if err != nil { + t.Fatal(err) + } + cm := chain.NewManager(store, genesisState) + es := testutil.NewElementStateStore(t, cm) + + testutil.MineBlocks(t, cm, addr, 20+int(n.MaturityDelay)) + es.Wait(t) + + cs := cm.TipState() + basis, sces := es.SiacoinElements() + for _, sce := range sces { + if sce.SiacoinOutput.Address == addr && sce.MaturityHeight <= cs.Index.Height { + return cm, basis, sce, sk, sp + } + } + t.Fatal("no spendable element found") + return nil, types.ChainIndex{}, types.SiacoinElement{}, nil, types.SpendPolicy{} + } + + // spend builds and signs a transaction spending parent and sending the + // remaining value (minus a fee) to dest. + spend := func(cm *chain.Manager, sk types.PrivateKey, sp types.SpendPolicy, parent types.SiacoinElement, dest types.Address) types.V2Transaction { + cs := cm.TipState() + txn := types.V2Transaction{ + SiacoinInputs: []types.V2SiacoinInput{ + {Parent: parent, SatisfiedPolicy: types.SatisfiedPolicy{Policy: sp}}, + }, + MinerFee: types.Siacoins(1), + SiacoinOutputs: []types.SiacoinOutput{ + {Address: dest, Value: parent.SiacoinOutput.Value.Sub(types.Siacoins(1))}, + }, + } + txn.SiacoinInputs[0].SatisfiedPolicy.Signatures = []types.Signature{sk.SignHash(cs.InputSigHash(txn))} + return txn + } + + t.Run("removes dependent transactions", func(t *testing.T) { + cm, basis, parent, sk, sp := setup(t) + parentTxn := spend(cm, sk, sp, parent, sp.Address()) + childTxn := spend(cm, sk, sp, parentTxn.EphemeralSiacoinOutput(0), types.VoidAddress) + if _, err := cm.AddV2PoolTransactions(basis, []types.V2Transaction{parentTxn, childTxn}); err != nil { + t.Fatal(err) + } else if n := len(cm.V2PoolTransactions()); n != 2 { + t.Fatalf("expected 2 pool transactions, got %d", n) + } + + // removing the parent must also drop the child that depends on it + cm.RemoveV2PoolTransactions([]types.TransactionID{parentTxn.ID()}) + if n := len(cm.V2PoolTransactions()); n != 0 { + t.Fatalf("expected empty pool after removing parent, got %d", n) + } + }) + + t.Run("keeps independent transactions", func(t *testing.T) { + cm, basis, parent, sk, sp := setup(t) + parentTxn := spend(cm, sk, sp, parent, sp.Address()) + childTxn := spend(cm, sk, sp, parentTxn.EphemeralSiacoinOutput(0), types.VoidAddress) + if _, err := cm.AddV2PoolTransactions(basis, []types.V2Transaction{parentTxn, childTxn}); err != nil { + t.Fatal(err) + } + + // removing only the child must leave the parent in the pool + cm.RemoveV2PoolTransactions([]types.TransactionID{childTxn.ID()}) + if _, ok := cm.V2PoolTransaction(parentTxn.ID()); !ok { + t.Fatal("expected parent to remain in pool") + } else if _, ok := cm.V2PoolTransaction(childTxn.ID()); ok { + t.Fatal("expected child to be removed from pool") + } + }) + + t.Run("no-op when transaction not in pool", func(t *testing.T) { + cm, basis, parent, sk, sp := setup(t) + txn := spend(cm, sk, sp, parent, types.VoidAddress) + if _, err := cm.AddV2PoolTransactions(basis, []types.V2Transaction{txn}); err != nil { + t.Fatal(err) + } + + var notified int + cancel := cm.OnPoolChange(func() { notified++ }) + defer cancel() + + // removing an unknown transaction must not change the pool or notify + cm.RemoveV2PoolTransactions([]types.TransactionID{{1, 2, 3}}) + if n := len(cm.V2PoolTransactions()); n != 1 { + t.Fatalf("expected pool to be unchanged, got %d", n) + } else if notified != 0 { + t.Fatalf("expected no notification for no-op removal, got %d", notified) + } + + // removing a known transaction must notify listeners + cm.RemoveV2PoolTransactions([]types.TransactionID{txn.ID()}) + if notified != 1 { + t.Fatalf("expected 1 notification, got %d", notified) + } else if n := len(cm.V2PoolTransactions()); n != 0 { + t.Fatalf("expected empty pool, got %d", n) + } + }) + + t.Run("frees inputs for reuse", func(t *testing.T) { + cm, basis, parent, sk, sp := setup(t) + first := spend(cm, sk, sp, parent, sp.Address()) + if _, err := cm.AddV2PoolTransactions(basis, []types.V2Transaction{first}); err != nil { + t.Fatal(err) + } + + // a different transaction reusing the same input conflicts while the + // first transaction is still in the pool + second := spend(cm, sk, sp, parent, types.VoidAddress) + if _, err := cm.AddV2PoolTransactions(basis, []types.V2Transaction{second}); err == nil { + t.Fatal("expected double-spend conflict while first transaction is in the pool") + } + + // removing the first transaction frees the input so it can be reused + cm.RemoveV2PoolTransactions([]types.TransactionID{first.ID()}) + if _, err := cm.AddV2PoolTransactions(basis, []types.V2Transaction{second}); err != nil { + t.Fatalf("expected to reuse freed input after removal, got %v", err) + } + }) +} From 9fcfc9eb8bf903fbfe44a96966e3ae5288b3686b Mon Sep 17 00:00:00 2001 From: Chris Schinnerl <3903476+ChrisSchinnerl@users.noreply.github.com> Date: Wed, 17 Jun 2026 10:56:11 +0200 Subject: [PATCH 3/6] add regression test --- rhp/v4/rpc_test.go | 133 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/rhp/v4/rpc_test.go b/rhp/v4/rpc_test.go index 01efae36..b0ebc9dc 100644 --- a/rhp/v4/rpc_test.go +++ b/rhp/v4/rpc_test.go @@ -14,6 +14,7 @@ import ( "slices" "strings" "sync" + "sync/atomic" "testing" "time" @@ -1369,6 +1370,138 @@ func TestRPCRenew(t *testing.T) { }) } +// failingContractor wraps an EphemeralContractor and can be configured to fail +// AddV2Contract and RenewV2Contract, simulating a host that fails to persist a +// contract after the transaction set has already been added to the pool but +// before it is broadcast. +type failingContractor struct { + *testutil.EphemeralContractor + failAdd atomic.Bool + failRenew atomic.Bool +} + +func (fc *failingContractor) AddV2Contract(ts rhp4.TransactionSet, usage proto4.Usage) error { + if fc.failAdd.Load() { + return errors.New("injected formation failure") + } + return fc.EphemeralContractor.AddV2Contract(ts, usage) +} + +func (fc *failingContractor) RenewV2Contract(ts rhp4.TransactionSet, usage proto4.Usage) error { + if fc.failRenew.Load() { + return errors.New("injected renewal failure") + } + return fc.EphemeralContractor.RenewV2Contract(ts, usage) +} + +// TestRPCRenewRecoverAfterPoolAdd ensures that when the host fails to persist a +// renewal after the renewal set has already been added to the transaction pool, +// the transaction is removed from the pool so that the contract is not left +// resolved and a subsequent renewal can succeed. +func TestRPCRenewRecoverAfterPoolAdd(t *testing.T) { + n, genesis := testutil.V2Network() + hostKey, renterKey := types.GeneratePrivateKey(), types.GeneratePrivateKey() + cm, w := startTestNode(t, n, genesis) + + // fund the wallet + mineAndSync(t, cm, w.Address(), int(n.MaturityDelay+20), w) + + sr := testutil.NewEphemeralSettingsReporter() + sr.Update(proto4.HostSettings{ + Release: "test", + AcceptingContracts: true, + WalletAddress: w.Address(), + MaxCollateral: types.Siacoins(10000), + MaxContractDuration: 1000, + RemainingStorage: 100 * proto4.SectorSize, + TotalStorage: 100 * proto4.SectorSize, + Prices: proto4.HostPrices{ + ContractPrice: types.Siacoins(1).Div64(5), // 0.2 SC + StoragePrice: types.NewCurrency64(100), // 100 H / byte / block + IngressPrice: types.NewCurrency64(100), // 100 H / byte + EgressPrice: types.NewCurrency64(100), // 100 H / byte + Collateral: types.NewCurrency64(200), + }, + }) + ss := testutil.NewEphemeralSectorStore() + c := &failingContractor{EphemeralContractor: testutil.NewEphemeralContractor(cm)} + + transport := testRenterHostPairSiaMux(t, hostKey, cm, w, c, sr, ss, zap.NewNop()) + + settings, err := rhp4.RPCSettings(context.Background(), transport) + if err != nil { + t.Fatal(err) + } + fundAndSign := &fundAndSign{w, renterKey} + + // form a contract and confirm it + formResult, err := rhp4.RPCFormContract(context.Background(), transport, cm, fundAndSign, cm.TipState(), settings.Prices, hostKey.PublicKey(), settings.WalletAddress, proto4.RPCFormContractParams{ + RenterPublicKey: renterKey.PublicKey(), + RenterAddress: w.Address(), + Allowance: types.Siacoins(100), + Collateral: types.Siacoins(200), + ProofHeight: cm.Tip().Height + 50, + }) + if err != nil { + t.Fatal(err) + } + if _, err := cm.AddV2PoolTransactions(formResult.FormationSet.Basis, formResult.FormationSet.Transactions); err != nil { + t.Fatal(err) + } + mineAndSync(t, cm, types.VoidAddress, 10, w, c) + + revision := formResult.Contract + renewParams := proto4.RPCRenewContractParams{ + ContractID: revision.ID, + Allowance: types.Siacoins(150), + Collateral: types.Siacoins(300), + ProofHeight: revision.Revision.ProofHeight + 10, + } + + // the pool should be empty now that the formation is confirmed + if n := len(cm.V2PoolTransactions()); n != 0 { + t.Fatalf("expected empty pool before renewal, got %d", n) + } + + // configure the host to fail persisting the renewal; this happens after the + // renewal set is added to the pool but before it is broadcast + c.failRenew.Store(true) + if _, err := rhp4.RPCRenewContract(context.Background(), transport, cm, fundAndSign, cm.TipState(), settings.Prices, settings.WalletAddress, revision.Revision, renewParams); err == nil { + t.Fatal("expected renewal to fail") + } + + // the failed renewal must have been removed from the pool, leaving the + // parent contract unresolved and revisable + if n := len(cm.V2PoolTransactions()); n != 0 { + t.Fatalf("expected failed renewal to be removed from the pool, got %d", n) + } + rs, err := rhp4.RPCLatestRevision(context.Background(), transport, revision.ID) + if err != nil { + t.Fatal(err) + } else if rs.Renewed { + t.Fatal("expected contract to not be renewed after failed renewal") + } else if !rs.Revisable { + t.Fatal("expected contract to still be revisable after failed renewal") + } + + // retrying the renewal must now succeed + c.failRenew.Store(false) + renewResult, err := rhp4.RPCRenewContract(context.Background(), transport, cm, fundAndSign, cm.TipState(), settings.Prices, settings.WalletAddress, revision.Revision, renewParams) + if err != nil { + t.Fatalf("expected renewal to succeed after recovery, got %v", err) + } + if known, err := cm.AddV2PoolTransactions(renewResult.RenewalSet.Basis, renewResult.RenewalSet.Transactions); err != nil { + t.Fatal(err) + } else if !known { + t.Fatal("expected renewal set to be known") + } + if rs, err := rhp4.RPCLatestRevision(context.Background(), transport, revision.ID); err != nil { + t.Fatal(err) + } else if !rs.Renewed { + t.Fatal("expected contract to be renewed after successful retry") + } +} + func TestRPCTimeout(t *testing.T) { n, genesis := testutil.V2Network() cm, w := startTestNode(t, n, genesis) From edc0d7a4fcd3797b8fba3316eb3a0af895a2ed5f Mon Sep 17 00:00:00 2001 From: Chris Schinnerl <3903476+ChrisSchinnerl@users.noreply.github.com> Date: Wed, 17 Jun 2026 10:57:28 +0200 Subject: [PATCH 4/6] changeset --- ...at_wasnt_broadcast_during_failed_formationrenewrefresh.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/clean_up_orphaned_transaction_hat_wasnt_broadcast_during_failed_formationrenewrefresh.md diff --git a/.changeset/clean_up_orphaned_transaction_hat_wasnt_broadcast_during_failed_formationrenewrefresh.md b/.changeset/clean_up_orphaned_transaction_hat_wasnt_broadcast_during_failed_formationrenewrefresh.md new file mode 100644 index 00000000..80376293 --- /dev/null +++ b/.changeset/clean_up_orphaned_transaction_hat_wasnt_broadcast_during_failed_formationrenewrefresh.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +# Clean up orphaned transaction hat wasn't broadcast during failed formation/renew/refresh. From f3fe179f3b2dab92975612e6200bfb052a0da81d Mon Sep 17 00:00:00 2001 From: Chris Schinnerl <3903476+ChrisSchinnerl@users.noreply.github.com> Date: Wed, 17 Jun 2026 13:18:55 +0200 Subject: [PATCH 5/6] fix typo --- ...n_hat_wasnt_broadcast_during_failed_formationrenewrefresh.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/clean_up_orphaned_transaction_hat_wasnt_broadcast_during_failed_formationrenewrefresh.md b/.changeset/clean_up_orphaned_transaction_hat_wasnt_broadcast_during_failed_formationrenewrefresh.md index 80376293..00fe6fb9 100644 --- a/.changeset/clean_up_orphaned_transaction_hat_wasnt_broadcast_during_failed_formationrenewrefresh.md +++ b/.changeset/clean_up_orphaned_transaction_hat_wasnt_broadcast_during_failed_formationrenewrefresh.md @@ -2,4 +2,4 @@ default: patch --- -# Clean up orphaned transaction hat wasn't broadcast during failed formation/renew/refresh. +# Clean up orphaned transaction that wasn't broadcast during failed formation/renew/refresh From 43034aabc1974b3a6584e58d97cb2d9ff1fbf55a Mon Sep 17 00:00:00 2001 From: Chris Schinnerl <3903476+ChrisSchinnerl@users.noreply.github.com> Date: Wed, 17 Jun 2026 13:36:27 +0200 Subject: [PATCH 6/6] address comments --- chain/manager.go | 1 + rhp/v4/server.go | 24 ++++++++++++++++++------ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/chain/manager.go b/chain/manager.go index ccc5673a..0986a0b2 100644 --- a/chain/manager.go +++ b/chain/manager.go @@ -1473,6 +1473,7 @@ func (m *Manager) RemoveV2PoolTransactions(ids []types.TransactionID) { // force a full revalidation m.txpool.ms = nil m.txpool.medianFee = nil + m.txpool.weight = 0 m.revalidatePool() // release lock while notifying listeners diff --git a/rhp/v4/server.go b/rhp/v4/server.go index 454a9b12..6ba2ad20 100644 --- a/rhp/v4/server.go +++ b/rhp/v4/server.go @@ -864,10 +864,14 @@ func (s *Server) handleRPCFormContract(stream net.Conn) error { }, usage) if err != nil { return fmt.Errorf("failed to add contract: %w", err) - } else if err := s.wallet.BroadcastV2TransactionSet(basis, formationSet); err != nil { + } + // the contract is committed; do not release the inputs or remove the + // transaction from the pool even if the broadcast to peers fails (the + // wallet has stored the set for rebroadcast) + broadcast = true + if err := s.wallet.BroadcastV2TransactionSet(basis, formationSet); err != nil { return fmt.Errorf("failed to broadcast transaction set: %w", err) } - broadcast = true // set broadcast so the UTXOs will not be released if the renter happens to disconnect before receiving the last response // send the finalized transaction set to the renter return rhp4.WriteResponse(stream, &rhp4.RPCFormContractThirdResponse{ @@ -1052,10 +1056,14 @@ func (s *Server) handleRPCRefreshContract(stream net.Conn, partial bool) error { }, usage) if err != nil { return fmt.Errorf("failed to add contract: %w", err) - } else if err := s.wallet.BroadcastV2TransactionSet(basis, renewalSet); err != nil { + } + // the contract is committed; do not release the inputs or remove the + // transaction from the pool even if the broadcast to peers fails (the + // wallet has stored the set for rebroadcast) + broadcast = true + if err := s.wallet.BroadcastV2TransactionSet(basis, renewalSet); err != nil { return fmt.Errorf("failed to broadcast transaction set: %w", err) } - broadcast = true // set broadcast so the UTXOs will not be released if the renter happens to disconnect before receiving the last response // send the finalized transaction set to the renter return rhp4.WriteResponse(stream, &rhp4.RPCRefreshContractThirdResponse{ @@ -1235,10 +1243,14 @@ func (s *Server) handleRPCRenewContract(stream net.Conn) error { }, usage) if err != nil { return fmt.Errorf("failed to add contract: %w", err) - } else if err := s.wallet.BroadcastV2TransactionSet(basis, renewalSet); err != nil { + } + // the contract is committed; do not release the inputs or remove the + // transaction from the pool even if the broadcast to peers fails (the + // wallet has stored the set for rebroadcast) + broadcast = true + if err := s.wallet.BroadcastV2TransactionSet(basis, renewalSet); err != nil { return fmt.Errorf("failed to broadcast transaction set: %w", err) } - broadcast = true // set broadcast so the UTXOs will not be released if the renter happens to disconnect before receiving the last response // send the finalized transaction set to the renter return rhp4.WriteResponse(stream, &rhp4.RPCRenewContractThirdResponse{