Merge pull request #1659 from guggero/itest-fixes

integration: optimize harness for better itest control, restore bitcoind compatibility

btcjson/chainsvrcmds.go

@@ -11,6 +11,7 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"reflect"
 
 	"github.com/btcsuite/btcd/wire"
 )
@@ -819,11 +820,60 @@ func NewSearchRawTransactionsCmd(address string, verbose, skip, count *int, vinE
 	}
 }
 
+// AllowHighFeesOrMaxFeeRate defines a type that can either be the legacy
+// allowhighfees boolean field or the new maxfeerate int field.
+type AllowHighFeesOrMaxFeeRate struct {
+	Value interface{}
+}
+
+// String returns the string representation of this struct, used for printing
+// the marshaled default value in the help text.
+func (a AllowHighFeesOrMaxFeeRate) String() string {
+	b, _ := a.MarshalJSON()
+	return string(b)
+}
+
+// MarshalJSON implements the json.Marshaler interface
+func (a AllowHighFeesOrMaxFeeRate) MarshalJSON() ([]byte, error) {
+	// The default value is false which only works with the legacy versions.
+	if a.Value == nil ||
+		(reflect.ValueOf(a.Value).Kind() == reflect.Ptr &&
+			reflect.ValueOf(a.Value).IsNil()) {
+
+		return json.Marshal(false)
+	}
+
+	return json.Marshal(a.Value)
+}
+
+// UnmarshalJSON implements the json.Unmarshaler interface
+func (a *AllowHighFeesOrMaxFeeRate) UnmarshalJSON(data []byte) error {
+	if len(data) == 0 {
+		return nil
+	}
+
+	var unmarshalled interface{}
+	if err := json.Unmarshal(data, &unmarshalled); err != nil {
+		return err
+	}
+
+	switch v := unmarshalled.(type) {
+	case bool:
+		a.Value = Bool(v)
+	case float64:
+		a.Value = Int32(int32(v))
+	default:
+		return fmt.Errorf("invalid allowhighfees or maxfeerate value: "+
+			"%v", unmarshalled)
+	}
+
+	return nil
+}
+
 // SendRawTransactionCmd defines the sendrawtransaction JSON-RPC command.
 type SendRawTransactionCmd struct {
-	HexTx         string
-	AllowHighFees *bool `jsonrpcdefault:"false"`
-	MaxFeeRate    *int32
+	HexTx      string
+	FeeSetting *AllowHighFeesOrMaxFeeRate `jsonrpcdefault:"false"`
 }
 
 // NewSendRawTransactionCmd returns a new instance which can be used to issue a
@@ -833,8 +883,10 @@ type SendRawTransactionCmd struct {
 // for optional parameters will use the default value.
 func NewSendRawTransactionCmd(hexTx string, allowHighFees *bool) *SendRawTransactionCmd {
 	return &SendRawTransactionCmd{
-		HexTx:         hexTx,
-		AllowHighFees: allowHighFees,
+		HexTx: hexTx,
+		FeeSetting: &AllowHighFeesOrMaxFeeRate{
+			Value: allowHighFees,
+		},
 	}
 }
 
@@ -844,8 +896,10 @@ func NewSendRawTransactionCmd(hexTx string, allowHighFees *bool) *SendRawTransac
 // A 0 maxFeeRate indicates that a maximum fee rate won't be enforced.
 func NewBitcoindSendRawTransactionCmd(hexTx string, maxFeeRate int32) *SendRawTransactionCmd {
 	return &SendRawTransactionCmd{
-		HexTx:      hexTx,
-		MaxFeeRate: &maxFeeRate,
+		HexTx: hexTx,
+		FeeSetting: &AllowHighFeesOrMaxFeeRate{
+			Value: &maxFeeRate,
+		},
 	}
 }
 

btcjson/chainsvrcmds_test.go

@@ -1224,29 +1224,49 @@ func TestChainSvrCmds(t *testing.T) {
 		{
 			name: "sendrawtransaction",
 			newCmd: func() (interface{}, error) {
-				return btcjson.NewCmd("sendrawtransaction", "1122")
+				return btcjson.NewCmd("sendrawtransaction", "1122", &btcjson.AllowHighFeesOrMaxFeeRate{})
 			},
 			staticCmd: func() interface{} {
 				return btcjson.NewSendRawTransactionCmd("1122", nil)
 			},
-			marshalled: `{"jsonrpc":"1.0","method":"sendrawtransaction","params":["1122"],"id":1}`,
+			marshalled: `{"jsonrpc":"1.0","method":"sendrawtransaction","params":["1122",false],"id":1}`,
 			unmarshalled: &btcjson.SendRawTransactionCmd{
-				HexTx:         "1122",
-				AllowHighFees: btcjson.Bool(false),
+				HexTx: "1122",
+				FeeSetting: &btcjson.AllowHighFeesOrMaxFeeRate{
+					Value: btcjson.Bool(false),
+				},
 			},
 		},
 		{
 			name: "sendrawtransaction optional",
 			newCmd: func() (interface{}, error) {
-				return btcjson.NewCmd("sendrawtransaction", "1122", false)
+				return btcjson.NewCmd("sendrawtransaction", "1122", &btcjson.AllowHighFeesOrMaxFeeRate{Value: btcjson.Bool(false)})
 			},
 			staticCmd: func() interface{} {
 				return btcjson.NewSendRawTransactionCmd("1122", btcjson.Bool(false))
 			},
 			marshalled: `{"jsonrpc":"1.0","method":"sendrawtransaction","params":["1122",false],"id":1}`,
 			unmarshalled: &btcjson.SendRawTransactionCmd{
-				HexTx:         "1122",
-				AllowHighFees: btcjson.Bool(false),
+				HexTx: "1122",
+				FeeSetting: &btcjson.AllowHighFeesOrMaxFeeRate{
+					Value: btcjson.Bool(false),
+				},
+			},
+		},
+		{
+			name: "sendrawtransaction optional, bitcoind >= 0.19.0",
+			newCmd: func() (interface{}, error) {
+				return btcjson.NewCmd("sendrawtransaction", "1122", &btcjson.AllowHighFeesOrMaxFeeRate{Value: btcjson.Int32(1234)})
+			},
+			staticCmd: func() interface{} {
+				return btcjson.NewBitcoindSendRawTransactionCmd("1122", 1234)
+			},
+			marshalled: `{"jsonrpc":"1.0","method":"sendrawtransaction","params":["1122",1234],"id":1}`,
+			unmarshalled: &btcjson.SendRawTransactionCmd{
+				HexTx: "1122",
+				FeeSetting: &btcjson.AllowHighFeesOrMaxFeeRate{
+					Value: btcjson.Int32(1234),
+				},
 			},
 		},
 		{

integration/bip0009_test.go

@@ -129,7 +129,7 @@ func assertSoftForkStatus(r *rpctest.Harness, t *testing.T, forkKey string, stat
 // specific soft fork deployment to test.
 func testBIP0009(t *testing.T, forkKey string, deploymentID uint32) {
 	// Initialize the primary mining node with only the genesis block.
-	r, err := rpctest.New(&chaincfg.RegressionNetParams, nil, nil)
+	r, err := rpctest.New(&chaincfg.RegressionNetParams, nil, nil, "")
 	if err != nil {
 		t.Fatalf("unable to create primary harness: %v", err)
 	}
@@ -320,7 +320,7 @@ func TestBIP0009Mining(t *testing.T) {
 	t.Parallel()
 
 	// Initialize the primary mining node with only the genesis block.
-	r, err := rpctest.New(&chaincfg.SimNetParams, nil, nil)
+	r, err := rpctest.New(&chaincfg.SimNetParams, nil, nil, "")
 	if err != nil {
 		t.Fatalf("unable to create primary harness: %v", err)
 	}

integration/csv_fork_test.go

@@ -109,7 +109,7 @@ func TestBIP0113Activation(t *testing.T) {
 	t.Parallel()
 
 	btcdCfg := []string{"--rejectnonstd"}
-	r, err := rpctest.New(&chaincfg.SimNetParams, nil, btcdCfg)
+	r, err := rpctest.New(&chaincfg.SimNetParams, nil, btcdCfg, "")
 	if err != nil {
 		t.Fatal("unable to create primary harness: ", err)
 	}
@@ -405,7 +405,7 @@ func TestBIP0068AndBIP0112Activation(t *testing.T) {
 	// relative lock times.
 
 	btcdCfg := []string{"--rejectnonstd"}
-	r, err := rpctest.New(&chaincfg.SimNetParams, nil, btcdCfg)
+	r, err := rpctest.New(&chaincfg.SimNetParams, nil, btcdCfg, "")
 	if err != nil {
 		t.Fatal("unable to create primary harness: ", err)
 	}

integration/rpcserver_test.go

@@ -109,7 +109,9 @@ func TestMain(m *testing.M) {
 	// ensure that non-standard transactions aren't accepted into the
 	// mempool or relayed.
 	btcdCfg := []string{"--rejectnonstd"}
-	primaryHarness, err = rpctest.New(&chaincfg.SimNetParams, nil, btcdCfg)
+	primaryHarness, err = rpctest.New(
+		&chaincfg.SimNetParams, nil, btcdCfg, "",
+	)
 	if err != nil {
 		fmt.Println("unable to create primary harness: ", err)
 		os.Exit(1)

integration/rpctest/node.go

@@ -41,10 +41,18 @@ type nodeConfig struct {
 }
 
 // newConfig returns a newConfig with all default values.
-func newConfig(prefix, certFile, keyFile string, extra []string) (*nodeConfig, error) {
-	btcdPath, err := btcdExecutablePath()
-	if err != nil {
-		btcdPath = "btcd"
+func newConfig(prefix, certFile, keyFile string, extra []string,
+	customExePath string) (*nodeConfig, error) {
+
+	var btcdPath string
+	if customExePath != "" {
+		btcdPath = customExePath
+	} else {
+		var err error
+		btcdPath, err = btcdExecutablePath()
+		if err != nil {
+			btcdPath = "btcd"
+		}
 	}
 
 	a := &nodeConfig{

integration/rpctest/rpc_harness.go

@@ -35,6 +35,14 @@ const (
 	// BlockVersion is the default block version used when generating
 	// blocks.
 	BlockVersion = 4
+
+	// DefaultMaxConnectionRetries is the default number of times we re-try
+	// to connect to the node after starting it.
+	DefaultMaxConnectionRetries = 20
+
+	// DefaultConnectionRetryTimeout is the default duration we wait between
+	// two connection attempts.
+	DefaultConnectionRetryTimeout = 50 * time.Millisecond
 )
 
 var (
@@ -49,6 +57,13 @@ var (
 
 	// Used to protest concurrent access to above declared variables.
 	harnessStateMtx sync.RWMutex
+
+	// ListenAddressGenerator is a function that is used to generate two
+	// listen addresses (host:port), one for the P2P listener and one for
+	// the RPC listener. This is exported to allow overwriting of the
+	// default behavior which isn't very concurrency safe (just selecting
+	// a random port can produce collisions and therefore flakes).
+	ListenAddressGenerator = generateListeningAddresses
 )
 
 // HarnessTestCase represents a test-case which utilizes an instance of the
@@ -69,15 +84,22 @@ type Harness struct {
 	// to.
 	ActiveNet *chaincfg.Params
 
+	// MaxConnRetries is the maximum number of times we re-try to connect to
+	// the node after starting it.
+	MaxConnRetries int
+
+	// ConnectionRetryTimeout is the duration we wait between two connection
+	// attempts.
+	ConnectionRetryTimeout time.Duration
+
 	Node     *rpcclient.Client
 	node     *node
 	handlers *rpcclient.NotificationHandlers
 
 	wallet *memWallet
 
-	testNodeDir    string
-	maxConnRetries int
-	nodeNum        int
+	testNodeDir string
+	nodeNum     int
 
 	sync.Mutex
 }
@@ -85,11 +107,12 @@ type Harness struct {
 // New creates and initializes new instance of the rpc test harness.
 // Optionally, websocket handlers and a specified configuration may be passed.
 // In the case that a nil config is passed, a default configuration will be
-// used.
+// used. If a custom btcd executable is specified, it will be used to start the
+// harness node. Otherwise a new binary is built on demand.
 //
 // NOTE: This function is safe for concurrent access.
 func New(activeNet *chaincfg.Params, handlers *rpcclient.NotificationHandlers,
-	extraArgs []string) (*Harness, error) {
+	extraArgs []string, customExePath string) (*Harness, error) {
 
 	harnessStateMtx.Lock()
 	defer harnessStateMtx.Unlock()
@@ -135,13 +158,15 @@ func New(activeNet *chaincfg.Params, handlers *rpcclient.NotificationHandlers,
 	miningAddr := fmt.Sprintf("--miningaddr=%s", wallet.coinbaseAddr)
 	extraArgs = append(extraArgs, miningAddr)
 
-	config, err := newConfig("rpctest", certFile, keyFile, extraArgs)
+	config, err := newConfig(
+		"rpctest", certFile, keyFile, extraArgs, customExePath,
+	)
 	if err != nil {
 		return nil, err
 	}
 
 	// Generate p2p+rpc listening addresses.
-	config.listen, config.rpcListen = generateListeningAddresses()
+	config.listen, config.rpcListen = ListenAddressGenerator()
 
 	// Create the testing node bounded to the simnet.
 	node, err := newNode(config, nodeTestData)
@@ -181,13 +206,14 @@ func New(activeNet *chaincfg.Params, handlers *rpcclient.NotificationHandlers,
 	}
 
 	h := &Harness{
-		handlers:       handlers,
-		node:           node,
-		maxConnRetries: 20,
-		testNodeDir:    nodeTestData,
-		ActiveNet:      activeNet,
-		nodeNum:        nodeNum,
-		wallet:         wallet,
+		handlers:               handlers,
+		node:                   node,
+		MaxConnRetries:         DefaultMaxConnectionRetries,
+		ConnectionRetryTimeout: DefaultConnectionRetryTimeout,
+		testNodeDir:            nodeTestData,
+		ActiveNet:              activeNet,
+		nodeNum:                nodeNum,
+		wallet:                 wallet,
 	}
 
 	// Track this newly created test instance within the package level
@@ -303,9 +329,9 @@ func (h *Harness) connectRPCClient() error {
 	var err error
 
 	rpcConf := h.node.config.rpcConnConfig()
-	for i := 0; i < h.maxConnRetries; i++ {
+	for i := 0; i < h.MaxConnRetries; i++ {
 		if client, err = rpcclient.New(&rpcConf, h.handlers); err != nil {
-			time.Sleep(time.Duration(i) * 50 * time.Millisecond)
+			time.Sleep(time.Duration(i) * h.ConnectionRetryTimeout)
 			continue
 		}
 		break

integration/rpctest/rpc_harness_test.go

@@ -105,7 +105,7 @@ func assertConnectedTo(t *testing.T, nodeA *Harness, nodeB *Harness) {
 
 func testConnectNode(r *Harness, t *testing.T) {
 	// Create a fresh test harness.
-	harness, err := New(&chaincfg.SimNetParams, nil, nil)
+	harness, err := New(&chaincfg.SimNetParams, nil, nil, "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -153,7 +153,7 @@ func testActiveHarnesses(r *Harness, t *testing.T) {
 	numInitialHarnesses := len(ActiveHarnesses())
 
 	// Create a single test harness.
-	harness1, err := New(&chaincfg.SimNetParams, nil, nil)
+	harness1, err := New(&chaincfg.SimNetParams, nil, nil, "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -181,7 +181,7 @@ func testJoinMempools(r *Harness, t *testing.T) {
 	// Create a local test harness with only the genesis block.  The nodes
 	// will be synced below so the same transaction can be sent to both
 	// nodes without it being an orphan.
-	harness, err := New(&chaincfg.SimNetParams, nil, nil)
+	harness, err := New(&chaincfg.SimNetParams, nil, nil, "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -281,7 +281,7 @@ func testJoinMempools(r *Harness, t *testing.T) {
 func testJoinBlocks(r *Harness, t *testing.T) {
 	// Create a second harness with only the genesis block so it is behind
 	// the main harness.
-	harness, err := New(&chaincfg.SimNetParams, nil, nil)
+	harness, err := New(&chaincfg.SimNetParams, nil, nil, "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -469,7 +469,7 @@ func testGenerateAndSubmitBlockWithCustomCoinbaseOutputs(r *Harness,
 func testMemWalletReorg(r *Harness, t *testing.T) {
 	// Create a fresh harness, we'll be using the main harness to force a
 	// re-org on this local harness.
-	harness, err := New(&chaincfg.SimNetParams, nil, nil)
+	harness, err := New(&chaincfg.SimNetParams, nil, nil, "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -566,7 +566,7 @@ const (
 
 func TestMain(m *testing.M) {
 	var err error
-	mainHarness, err = New(&chaincfg.SimNetParams, nil, nil)
+	mainHarness, err = New(&chaincfg.SimNetParams, nil, nil, "")
 	if err != nil {
 		fmt.Println("unable to create main harness: ", err)
 		os.Exit(1)

rpcserverhelp.go

@@ -561,11 +561,11 @@ var helpDescsEnUS = map[string]string{
 	"searchrawtransactions--result0":    "Hex-encoded serialized transaction",
 
 	// SendRawTransactionCmd help.
-	"sendrawtransaction--synopsis":     "Submits the serialized, hex-encoded transaction to the local peer and relays it to the network.",
-	"sendrawtransaction-hextx":         "Serialized, hex-encoded signed transaction",
-	"sendrawtransaction-allowhighfees": "Whether or not to allow insanely high fees (btcd does not yet implement this parameter, so it has no effect)",
-	"sendrawtransaction-maxfeerate":    "Used by bitcoind on or after v0.19.0",
-	"sendrawtransaction--result0":      "The hash of the transaction",
+	"sendrawtransaction--synopsis":    "Submits the serialized, hex-encoded transaction to the local peer and relays it to the network.",
+	"sendrawtransaction-hextx":        "Serialized, hex-encoded signed transaction",
+	"sendrawtransaction-feesetting":   "Whether or not to allow insanely high fees in bitcoind < v0.19.0 or the max fee rate for bitcoind v0.19.0 and later (btcd does not yet implement this parameter, so it has no effect)",
+	"sendrawtransaction--result0":     "The hash of the transaction",
+	"allowhighfeesormaxfeerate-value": "Either the boolean value for the allowhighfees parameter in bitcoind < v0.19.0 or the numerical value for the maxfeerate field in bitcoind v0.19.0 and later",
 
 	// SetGenerateCmd help.
 	"setgenerate--synopsis":    "Set the server to generate coins (mine) or not.",