rpcclient: preserve transactions during mempool accept validation
What the developer wrote
Authored by Olaoluwa Osuntokun
rpcclient: preserve transactions during mempool accept validation
In this commit, we validate serialized transactions with a temporary MsgTx instead of decoding back into the caller's object. TestMempoolAcceptAsync now preserves the supplied transactions on both successful validation and local decode errors.
The regression checks transaction contents and input/output identity, rejection without an outbound request, and the transaction bytes and fee limit queued for a valid request.
What changed, and why it matters
This commit fixes a bug in btcd's RPC client where a function that checks whether transactions would be accepted by the mempool was accidentally modifying the caller's transaction data during its internal validation. The fix makes the function decode into a temporary copy instead of the caller's original object, so users' transactions remain unchanged. The included tests verify the original transaction is preserved on both success and failure paths.
Treat as a low-severity reliability/defensive fix. Review other RPC client methods for similar in-place deserialization of caller-provided objects. No immediate incident response required unless downstream code was relying on the previous mutation behavior.
Security signals we found
Mutation of caller-owned input objects in a public API
Potential transaction data corruption or unexpected state change in downstream consumers
Regression test added covering both success and failure validation paths
Evidence from the diff
In rpcclient/rawtransactions.go, TestMempoolAcceptAsync previously called tx.Deserialize(buf) on each caller-supplied *wire.MsgTx to validate serialization before sending the RPC. Because Deserialize mutates the receiver, this could alter or corrupt the caller’s transaction object. The change introduces a local var decoded wire.MsgTx and calls decoded.Deserialize(buf), leaving the input txns untouched. A new regression test confirms identity preservation of inputs/outputs, correct rejection without outbound request on decode failure, and proper request payload on success.
Changed components
rpcclient/rawtransactions.gorpcclient/rawtransactions_test.goTestMempoolAcceptAsync functionInspect captured patch +68 / −1
### rpcclient/rawtransactions.go
@@ -994,7 +994,8 @@ func (c *Client) TestMempoolAcceptAsync(txns []*wire.MsgTx,
// NOTE: must be performed after buf.Bytes is copied above.
//
// TODO(yy): remove it once the above TODO is addressed.
- if err := tx.Deserialize(buf); err != nil {
+ var decoded wire.MsgTx
+ if err := decoded.Deserialize(buf); err != nil {
err = fmt.Errorf("%w: %v", ErrInvalidParam, err)
return newFutureError(err)
}
### rpcclient/rawtransactions_test.go
@@ -0,0 +1,66 @@
+package rpcclient
+
+import (
+ "bytes"
+ "encoding/hex"
+ "testing"
+
+ "github.com/btcsuite/btcd/btcjson"
+ "github.com/btcsuite/btcd/wire/v2"
+ "github.com/stretchr/testify/require"
+)
+
+// TestMempoolAcceptPreservesTransactions checks that local validation leaves
+// caller-owned transactions unchanged on both success and failure.
+func TestMempoolAcceptPreservesTransactions(t *testing.T) {
+ for _, valid := range []bool{false, true} {
+ name := "invalid"
+ if valid {
+ name = "valid"
+ }
+ t.Run(name, func(t *testing.T) {
+ client := &Client{
+ config: &ConnConfig{HTTPPostMode: true},
+ backendVersion: BitcoindPost25,
+ sendPostChan: make(chan *jsonRequest, 1),
+ shutdown: make(chan struct{}),
+ }
+ tx := wire.NewMsgTx(2)
+ if valid {
+ tx.AddTxIn(wire.NewTxIn(
+ &wire.OutPoint{}, []byte{0x01},
+ wire.TxWitness{{0x02, 0x03}},
+ ))
+ }
+ tx.AddTxOut(wire.NewTxOut(1, []byte{0x04, 0x05}))
+ tx.LockTime = 123
+ before := tx.Copy()
+ inputs := tx.TxIn
+ output := tx.TxOut[0]
+ var serialized bytes.Buffer
+ require.NoError(t, tx.Serialize(&serialized))
+
+ future := client.TestMempoolAcceptAsync([]*wire.MsgTx{tx}, 0.1)
+ require.Equal(t, before, tx)
+ require.Same(t, output, tx.TxOut[0])
+ if !valid {
+ require.Empty(t, client.sendPostChan)
+ require.Len(t, future, 1)
+ _, err := future.Receive()
+ require.ErrorIs(t, err, ErrInvalidParam)
+ return
+ }
+
+ require.Same(t, inputs[0], tx.TxIn[0])
+ require.Len(t, client.sendPostChan, 1)
+ request := <-client.sendPostChan
+ require.Equal(t, "testmempoolaccept", request.method)
+ command := request.cmd.(*btcjson.TestMempoolAcceptCmd)
+ require.Equal(t, []string{hex.EncodeToString(serialized.Bytes())}, command.RawTxns)
+ require.Equal(t, btcjson.BTCPerkvB(0.1), command.MaxFeeRate)
+ request.responseChan <- &Response{result: []byte("[]")}
+ _, err := future.Receive()
+ require.NoError(t, err)
+ })
+ }
+}Why this scored 34/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.