Skip to content

Commit

Permalink
server/asset: fail fast if txindex is not enabled for btc clones
Browse files Browse the repository at this point in the history
This ensures txindex is enabled by fetching information for a coinbase
transaction of a given block which is essential for DEX activities. It is
fatal for btc clone markets if this is not enabled since getrawtransaction
will fail.
  • Loading branch information
ukane-philemon authored Nov 29, 2022
1 parent ce6abbf commit cf6d205
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 21 deletions.
1 change: 0 additions & 1 deletion dex/networks/btc/script_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,6 @@ func TestIsDust(t *testing.T) {
if res != test.isDust {
t.Fatalf("Dust test '%s' failed: want %v got %v",
test.name, test.isDust, res)
continue
}
}
}
Expand Down
1 change: 0 additions & 1 deletion dex/networks/dcr/script_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,6 @@ func TestIsDust(t *testing.T) {
if res != test.isDust {
t.Fatalf("Dust test '%s' failed: want %v got %v",
test.name, test.isDust, res)
continue
}
}
}
Expand Down
22 changes: 9 additions & 13 deletions server/asset/btc/btc.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,19 +368,15 @@ func (btc *Backend) Connect(ctx context.Context) (*sync.WaitGroup, error) {
}
}

if txindex, err := btc.node.checkTxIndex(); err != nil {
if !isMethodNotFoundErr(err) {
btc.shutdown()
return nil, fmt.Errorf("%s getindexinfo check failed: %w", btc.name, err)
}
// Ignore and log err if getindexinfo method is not found.
// getindexinfo method is not currently supported by
// pre 0.21 versions of bitcoind, and some forks of
// bitcoin core (litecoin).
btc.log.Warnf("The getindexinfo RPC is unavailable. Please ensure txindex is enabled in the node config.")
} else if !txindex {
txindex, err := btc.node.checkTxIndex()
if err != nil {
btc.log.Warnf(`Please ensure txindex is enabled in the node config and you might need to re-index if txindex was not previously enabled for %s`, btc.name)
btc.shutdown()
return nil, fmt.Errorf("error checking txindex for %s: %w", btc.name, err)
}
if !txindex {
btc.shutdown()
return nil, fmt.Errorf("%s transaction index is not enabled. Please enable txindex in the node config", btc.name)
return nil, fmt.Errorf("%s transaction index is not enabled. Please enable txindex in the node config and you might need to re-index when you enable txindex", btc.name)
}

if _, err = btc.estimateFee(ctx); err != nil {
Expand Down Expand Up @@ -648,7 +644,7 @@ func (btc *Backend) TxData(coinID []byte) ([]byte, error) {
if isTxNotFoundErr(err) {
return nil, asset.CoinNotFoundError
}
return nil, fmt.Errorf("GetRawTransactionVerbose for txid %s: %w", txHash, err)
return nil, fmt.Errorf("GetRawTransaction for txid %s: %w", txHash, err)
}
return txB, nil
}
Expand Down
54 changes: 49 additions & 5 deletions server/asset/btc/rpcclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const (
methodGetIndexInfo = "getindexinfo"
methodGetBlockHeader = "getblockheader"
methodGetBlockStats = "getblockstats"
methodGetBlockHash = "getblockhash"

errNoCompetition = dex.ErrorKind("no competition")
errNoFeeRate = dex.ErrorKind("fee rate could not be estimated")
Expand Down Expand Up @@ -96,8 +97,8 @@ func (rc *RPCClient) GetBlockChainInfo() (*GetBlockchainInfoResult, error) {
return chainInfo, nil
}

// txIndexResult models the data returned from the getindexinfo command
// for txindex.
// txIndexResult models the data returned from the getindexinfo command for
// txindex.
// txIndexResult.Txindex is nil if the returned data is an empty json object.
type txIndexResult struct {
TxIndex *struct{} `json:"txindex"`
Expand All @@ -107,12 +108,55 @@ type txIndexResult struct {
func (rc *RPCClient) checkTxIndex() (bool, error) {
res := new(txIndexResult)
err := rc.call(methodGetIndexInfo, anylist{"txindex"}, res)
if err == nil {
// Return early if there is no error. bitcoind returns an empty json
// object if txindex is not enabled. It is safe to conclude txindex is
// enabled if res.Txindex is not nil.
return res.TxIndex != nil, nil
}

if !isMethodNotFoundErr(err) {
return false, err
}

// Using block at index 5 to retrieve a coinbase transaction and ensure
// txindex is enabled for pre 0.21 versions of bitcoind.
const blockIndex = 5
blockHash, err := rc.getBlockHash(blockIndex)
if err != nil {
return false, err
}

blockInfo, err := rc.GetBlockVerbose(blockHash)
if err != nil {
return false, err
}

if len(blockInfo.Tx) == 0 {
return false, fmt.Errorf("block %d does not have a coinbase transaction", blockIndex)
}

txHash, err := chainhash.NewHashFromStr(blockInfo.Tx[0])
if err != nil {
return false, err
}

// Retrieve coinbase transaction information.
txBytes, err := rc.GetRawTransaction(txHash)
if err != nil {
return false, err
}
// bitcoind returns an empty json object if txindex is not enabled.
// It is safe to conclude txindex is enabled if res.Txindex is not nil.
return res.TxIndex != nil, nil

return len(txBytes) != 0, nil
}

// getBlockHash fetches the block hash for the block at the given index.
func (rc *RPCClient) getBlockHash(index int64) (*chainhash.Hash, error) {
var blockHashStr string
if err := rc.call(methodGetBlockHash, anylist{index}, &blockHashStr); err != nil {
return nil, err
}
return chainhash.NewHashFromStr(blockHashStr)
}

// EstimateSmartFee requests the server to estimate a fee level.
Expand Down
2 changes: 1 addition & 1 deletion server/dex/dex.go
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ func NewDEX(ctx context.Context, cfg *DexConf) (*DEX, error) {
// Because the dexBalancer relies on the marketTunnels map, and NewMarket
// checks necessary balances for account-based assets using the dexBalancer,
// that means that each market can only query orders for the markets that
// were intitialized before it was, which is fine, but notable. The
// were initialized before it was, which is fine, but notable. The
// resulting behavior is that a user could have orders involving an
// account-based asset approved for re-booking on one market, but have
// orders rejected on a market involving the same asset created afterwards,
Expand Down

0 comments on commit cf6d205

Please sign in to comment.