Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Nov 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 61 additions & 7 deletions btcjson/chainsvrcmds.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"encoding/hex"
"encoding/json"
"fmt"
"reflect"

"github.com/btcsuite/btcd/wire"
)
Expand Down Expand Up @@ -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{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, clever way to handle this!

}

// 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
Expand All @@ -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,
},
}
}

Expand All @@ -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,
},
}
}

Expand Down
34 changes: 27 additions & 7 deletions btcjson/chainsvrcmds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
},
},
{
Expand Down
4 changes: 2 additions & 2 deletions integration/bip0009_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions integration/csv_fork_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 3 additions & 1 deletion integration/rpcserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 12 additions & 4 deletions integration/rpctest/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
58 changes: 42 additions & 16 deletions integration/rpctest/rpc_harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,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 (
Expand All @@ -58,6 +66,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
Expand All @@ -78,27 +93,35 @@ 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
}

// 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()
Expand Down Expand Up @@ -144,13 +167,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)
Expand Down Expand Up @@ -190,13 +215,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
Expand Down Expand Up @@ -312,9 +338,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
Expand Down
Loading