diff --git a/doc/release-notes.md b/doc/release-notes.md
--- a/doc/release-notes.md
+++ b/doc/release-notes.md
@@ -2,4 +2,5 @@
<https://download.bitcoinabc.org/0.18.4/>
-
+This release includes the following features and fixes:
+ - Fixes and issue with `getblocktemplate` returning fees and sigops in non-CTOR ordering after Nov 15th CTOR activation.
\ No newline at end of file
diff --git a/src/miner.h b/src/miner.h
--- a/src/miner.h
+++ b/src/miner.h
@@ -24,10 +24,19 @@
static const bool DEFAULT_PRINTPRIORITY = false;
+struct CBlockTemplateEntry {
+ CTransactionRef tx;
+ Amount fees;
+ int64_t sigOpCount;
+
+ CBlockTemplateEntry(CTransactionRef tx, Amount fees, int64_t sigOpCount)
+ : tx(tx), fees(fees), sigOpCount(sigOpCount){};
+};
+
struct CBlockTemplate {
CBlock block;
- std::vector<Amount> vTxFees;
- std::vector<int64_t> vTxSigOpsCount;
+
+ std::vector<CBlockTemplateEntry> entries;
};
// Container for tracking updates to ancestor feerate as we include (parent)
diff --git a/src/miner.cpp b/src/miner.cpp
--- a/src/miner.cpp
+++ b/src/miner.cpp
@@ -32,9 +32,6 @@
#include <queue>
#include <utility>
-#include <boost/thread.hpp>
-#include <boost/tuple/tuple.hpp>
-
//////////////////////////////////////////////////////////////////////////////
//
// BitcoinMiner
@@ -141,12 +138,8 @@
// Pointer for convenience.
pblock = &pblocktemplate->block;
- // Add dummy coinbase tx as first transaction.
- pblock->vtx.emplace_back();
- // updated at end
- pblocktemplate->vTxFees.push_back(-SATOSHI);
- // updated at end
- pblocktemplate->vTxSigOpsCount.push_back(-1);
+ // Add dummy coinbase tx as first transaction. It is updated at the end.
+ pblocktemplate->entries.emplace_back(CTransactionRef(), -SATOSHI, -1);
LOCK2(cs_main, mempool.cs);
CBlockIndex *pindexPrev = chainActive.Tip();
@@ -178,11 +171,11 @@
if (IsMagneticAnomalyEnabled(*config, pindexPrev)) {
// If magnetic anomaly is enabled, we make sure transaction are
// canonically ordered.
- std::sort(std::begin(pblock->vtx) + 1, std::end(pblock->vtx),
- [](const std::shared_ptr<const CTransaction> &a,
- const std::shared_ptr<const CTransaction> &b) -> bool {
- return a->GetId() < b->GetId();
- });
+ // FIXME: Use a zipped list. See T479
+ std::sort(std::begin(pblocktemplate->entries) + 1,
+ std::end(pblocktemplate->entries),
+ [](const CBlockTemplateEntry &a, const CBlockTemplateEntry &b)
+ -> bool { return a.tx->GetId() < b.tx->GetId(); });
}
int64_t nTime1 = GetTimeMicros();
@@ -208,8 +201,8 @@
<< std::vector<uint8_t>(MIN_TX_SIZE - coinbaseSize - 1);
}
- pblock->vtx[0] = MakeTransactionRef(coinbaseTx);
- pblocktemplate->vTxFees[0] = -1 * nFees;
+ pblocktemplate->entries[0].tx = MakeTransactionRef(coinbaseTx);
+ pblocktemplate->entries[0].fees = -1 * nFees;
uint64_t nSerializeSize =
GetSerializeSize(*pblock, SER_NETWORK, PROTOCOL_VERSION);
@@ -222,8 +215,15 @@
UpdateTime(pblock, *config, pindexPrev);
pblock->nBits = GetNextWorkRequired(pindexPrev, pblock, *config);
pblock->nNonce = 0;
- pblocktemplate->vTxSigOpsCount[0] = GetSigOpCountWithoutP2SH(
- *pblock->vtx[0], STANDARD_CHECKDATASIG_VERIFY_FLAGS);
+ pblocktemplate->entries[0].sigOpCount = GetSigOpCountWithoutP2SH(
+ *pblocktemplate->entries[0].tx, STANDARD_CHECKDATASIG_VERIFY_FLAGS);
+
+ // Copy all the transactions into the block
+ // FIXME: This should be removed as it is significant overhead.
+ // See T479
+ for (const CBlockTemplateEntry &tx : pblocktemplate->entries) {
+ pblock->vtx.push_back(tx.tx);
+ }
CValidationState state;
BlockValidationOptions validationOptions(false, false);
@@ -350,9 +350,8 @@
}
void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) {
- pblock->vtx.emplace_back(iter->GetSharedTx());
- pblocktemplate->vTxFees.push_back(iter->GetFee());
- pblocktemplate->vTxSigOpsCount.push_back(iter->GetSigOpCount());
+ pblocktemplate->entries.emplace_back(iter->GetSharedTx(), iter->GetFee(),
+ iter->GetSigOpCount());
nBlockSize += iter->GetTxSize();
++nBlockTx;
nBlockSigOps += iter->GetSigOpCount();
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -632,9 +632,10 @@
entry.pushKV("depends", deps);
int index_in_template = i - 1;
- entry.push_back(
- Pair("fee", pblocktemplate->vTxFees[index_in_template] / SATOSHI));
- int64_t nTxSigOps = pblocktemplate->vTxSigOpsCount[index_in_template];
+ entry.pushKV("fee",
+ pblocktemplate->entries[index_in_template].fees / SATOSHI);
+ int64_t nTxSigOps =
+ pblocktemplate->entries[index_in_template].sigOpCount;
entry.pushKV("sigops", nTxSigOps);
transactions.push_back(entry);
diff --git a/test/functional/abc-magnetic-anomaly-mining.py b/test/functional/abc-magnetic-anomaly-mining.py
new file mode 100755
--- /dev/null
+++ b/test/functional/abc-magnetic-anomaly-mining.py
@@ -0,0 +1,122 @@
+#!/usr/bin/env python3
+# Copyright (c) 2014-2016 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""
+Test that mining RPC continues to supply correct transaction metadata after
+the Nov 2018 protocol upgrade which engages canonical transaction ordering
+"""
+
+import time
+import random
+import decimal
+
+from test_framework.blocktools import create_coinbase
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import *
+
+
+class CTORMiningTest(BitcoinTestFramework):
+ def set_test_params(self):
+ # Setup two nodes so we can getblocktemplate
+ # it errors out if it is not connected to other nodes
+ self.num_nodes = 2
+ self.setup_clean_chain = True
+ self.block_heights = {}
+ self.tip = None
+ self.blocks = {}
+ self.mocktime = int(time.time()) - 600 * 100
+
+ extra_arg = ['-spendzeroconfchange=0', '-whitelist=127.0.0.1',
+ "-magneticanomalyactivationtime=%d" % self.mocktime,
+ "-replayprotectionactivationtime=%d" % (10 * self.mocktime)]
+ self.extra_args = [extra_arg, extra_arg]
+
+ def run_test(self):
+ mining_node = self.nodes[0]
+
+ # Helper for updating the times
+ def update_time():
+ mining_node.setmocktime(self.mocktime)
+ self.mocktime = self.mocktime + 600
+
+ mining_node.getnewaddress()
+
+ # Generate some unspent utxos and also
+ # activate magnetic anomaly
+ for x in range(150):
+ update_time()
+ mining_node.generate(1)
+
+ update_time()
+ unspent = mining_node.listunspent()
+
+ transactions = {}
+ # Spend all our coinbases
+ while len(unspent):
+ inputs = []
+ # Grab a random number of inputs
+ for _ in range(random.randrange(1, 5)):
+ txin = unspent.pop()
+ inputs.append({
+ 'txid': txin['txid'],
+ 'vout': 0 # This is a coinbase
+ })
+ if len(unspent) == 0:
+ break
+
+ outputs = {}
+ # Calculate a unique fee for this transaction
+ fee = decimal.Decimal(random.randint(
+ 1000, 2000)) / decimal.Decimal(1e8)
+ # Spend to the same number of outputs as inputs, so we can leave
+ # the amounts unchanged and avoid rounding errors.
+ #
+ # NOTE: There will be 1 sigop per output (which equals the number
+ # of inputs now). We need this randomization to ensure the
+ # numbers are properly following the transactions in the block
+ # template metadata
+ addr = ""
+ for _ in range(len(inputs)):
+ addr = mining_node.getnewaddress()
+ output = {
+ # 50 BCH per coinbase
+ addr: decimal.Decimal(50)
+ }
+ outputs.update(output)
+
+ # Take the fee off the last output to avoid rounding errors we
+ # need the exact fee later for assertions
+ outputs[addr] -= fee
+
+ rawtx = mining_node.createrawtransaction(inputs, outputs)
+ signedtx = mining_node.signrawtransaction(rawtx)
+ txid = mining_node.sendrawtransaction(signedtx['hex'])
+ # number of outputs is the same as the number of sigops in this
+ # case
+ transactions.update({txid: {'fee': fee, 'sigops': len(outputs)}})
+
+ tmpl = mining_node.getblocktemplate()
+ assert 'proposal' in tmpl['capabilities']
+ assert 'coinbasetxn' not in tmpl
+
+ # Check the template transaction metadata and ordering
+ last_txid = 0
+ for txn in tmpl['transactions'][1:]:
+ txid = txn['txid']
+ txnMetadata = transactions[txid]
+ expectedFeeSats = int(txnMetadata['fee'] * 10**8)
+ expectedSigOps = txnMetadata['sigops']
+
+ txid_decoded = int(txid, 16)
+
+ # Assert we got the expected metadata
+ assert(expectedFeeSats == txn['fee'])
+ assert(expectedSigOps == txn['sigops'])
+ # Assert transaction ids are in order
+ assert(last_txid == 0 or last_txid < txid_decoded)
+ last_txid = txid_decoded
+
+
+if __name__ == '__main__':
+ CTORMiningTest().main()
diff --git a/test/functional/timing.json b/test/functional/timing.json
--- a/test/functional/timing.json
+++ b/test/functional/timing.json
@@ -1,7 +1,7 @@
[
{
"name": "abandonconflict.py",
- "time": 13
+ "time": 16
},
{
"name": "abc-checkdatasig-activation.py",
@@ -9,31 +9,35 @@
},
{
"name": "abc-cmdline.py",
- "time": 9
+ "time": 8
},
{
"name": "abc-high_priority_transaction.py",
- "time": 12
+ "time": 10
},
{
"name": "abc-magnetic-anomaly-activation.py",
- "time": 7
+ "time": 6
+ },
+ {
+ "name": "abc-magnetic-anomaly-mining.py",
+ "time": 15
},
{
"name": "abc-mempool-accept-txn.py",
- "time": 4
+ "time": 5
},
{
"name": "abc-p2p-compactblocks.py",
- "time": 167
+ "time": 104
},
{
"name": "abc-p2p-fullblocktest.py",
- "time": 50
+ "time": 81
},
{
"name": "abc-parkedchain.py",
- "time": 3
+ "time": 4
},
{
"name": "abc-replay-protection.py",
@@ -45,47 +49,47 @@
},
{
"name": "abc-transaction-ordering.py",
- "time": 15
+ "time": 6
},
{
"name": "assumevalid.py",
- "time": 13
+ "time": 20
},
{
"name": "bip65-cltv-p2p.py",
- "time": 19
+ "time": 5
},
{
"name": "bip68-112-113-p2p.py",
- "time": 34
+ "time": 18
},
{
"name": "bip68-sequence.py",
+ "time": 18
+ },
+ {
+ "name": "bip68-sequence.py --magnetic-anomaly-time=1",
"time": 26
},
{
"name": "bipdersig-p2p.py",
- "time": 5
+ "time": 7
},
{
"name": "bitcoin_cli.py",
- "time": 13
+ "time": 4
},
{
"name": "blockchain.py",
- "time": 10
+ "time": 11
},
{
"name": "dbcrash.py",
- "time": 1257
+ "time": 756
},
{
"name": "decodescript.py",
- "time": 2
- },
- {
- "name": "deprecated_rpc.py",
- "time": 18
+ "time": 3
},
{
"name": "disablewallet.py",
@@ -93,15 +97,15 @@
},
{
"name": "disconnect_ban.py",
- "time": 9
+ "time": 8
},
{
"name": "example_test.py",
- "time": 5
+ "time": 4
},
{
"name": "fundrawtransaction.py",
- "time": 50
+ "time": 42
},
{
"name": "getblocktemplate_longpoll.py",
@@ -109,11 +113,11 @@
},
{
"name": "getchaintips.py",
- "time": 5
+ "time": 4
},
{
"name": "httpbasics.py",
- "time": 4
+ "time": 3
},
{
"name": "import-rescan.py",
@@ -121,163 +125,163 @@
},
{
"name": "importmulti.py",
- "time": 24
+ "time": 12
},
{
"name": "importprunedfunds.py",
- "time": 8
+ "time": 4
},
{
"name": "invalidateblock.py",
- "time": 10
+ "time": 9
},
{
"name": "invalidblockrequest.py",
- "time": 14
+ "time": 4
},
{
"name": "invalidtxrequest.py",
- "time": 5
+ "time": 4
},
{
"name": "keypool-topup.py",
- "time": 45
+ "time": 19
},
{
"name": "keypool.py",
- "time": 9
+ "time": 8
},
{
"name": "listsinceblock.py",
- "time": 4
+ "time": 5
},
{
"name": "listtransactions.py",
- "time": 10
+ "time": 11
},
{
"name": "maxuploadtarget.py",
- "time": 49
+ "time": 37
},
{
"name": "mempool_limit.py",
- "time": 7
+ "time": 5
},
{
"name": "mempool_packages.py",
- "time": 34
+ "time": 28
},
{
"name": "mempool_persist.py",
- "time": 21
+ "time": 16
},
{
"name": "mempool_reorg.py",
- "time": 28
+ "time": 4
},
{
"name": "mempool_resurrect_test.py",
- "time": 18
+ "time": 3
},
{
"name": "mempool_spendcoinbase.py",
- "time": 17
+ "time": 3
},
{
"name": "merkle_blocks.py",
- "time": 4
+ "time": 5
},
{
"name": "minchainwork.py",
- "time": 16
+ "time": 6
},
{
"name": "mining.py",
- "time": 18
+ "time": 3
},
{
"name": "multi_rpc.py",
- "time": 5
+ "time": 4
},
{
"name": "multiwallet.py",
- "time": 8
+ "time": 9
},
{
"name": "net.py",
- "time": 17
+ "time": 4
},
{
"name": "notifications.py",
- "time": 7
+ "time": 6
},
{
"name": "nulldummy.py",
- "time": 17
+ "time": 4
},
{
"name": "p2p-acceptblock.py",
- "time": 8
+ "time": 7
},
{
"name": "p2p-compactblocks.py",
- "time": 42
+ "time": 23
},
{
"name": "p2p-feefilter.py",
- "time": 25
+ "time": 63
},
{
"name": "p2p-fullblocktest.py",
- "time": 154
+ "time": 184
},
{
"name": "p2p-leaktests.py",
- "time": 24
+ "time": 7
},
{
"name": "p2p-mempool.py",
- "time": 16
+ "time": 3
},
{
"name": "p2p-timeouts.py",
- "time": 65
+ "time": 64
},
{
"name": "preciousblock.py",
- "time": 5
+ "time": 4
},
{
"name": "prioritise_transaction.py",
- "time": 10
+ "time": 6
},
{
"name": "proxy_test.py",
- "time": 5
+ "time": 4
},
{
"name": "pruning.py",
- "time": 1491
+ "time": 1040
},
{
"name": "rawtransactions.py",
- "time": 37
+ "time": 17
},
{
"name": "receivedby.py",
- "time": 13
+ "time": 6
},
{
"name": "reindex.py",
- "time": 26
+ "time": 14
},
{
"name": "resendwallettransactions.py",
- "time": 5
+ "time": 6
},
{
"name": "rest.py",
- "time": 9
+ "time": 11
},
{
"name": "rpcbind_test.py",
@@ -289,11 +293,11 @@
},
{
"name": "sendheaders.py",
- "time": 30
+ "time": 22
},
{
"name": "signmessages.py",
- "time": 2
+ "time": 3
},
{
"name": "signrawtransactions.py",
@@ -301,27 +305,27 @@
},
{
"name": "txn_clone.py",
- "time": 7
+ "time": 5
},
{
"name": "txn_clone.py --mineblock",
- "time": 8
+ "time": 6
},
{
"name": "txn_doublespend.py",
- "time": 6
+ "time": 5
},
{
"name": "txn_doublespend.py --mineblock",
- "time": 5
+ "time": 6
},
{
"name": "uptime.py",
- "time": 3
+ "time": 2
},
{
"name": "wallet-accounts.py",
- "time": 40
+ "time": 13
},
{
"name": "wallet-dump.py",
@@ -329,19 +333,19 @@
},
{
"name": "wallet-encryption.py",
- "time": 20
+ "time": 9
},
{
"name": "wallet-hd.py",
- "time": 126
+ "time": 111
},
{
"name": "wallet.py",
- "time": 44
+ "time": 56
},
{
"name": "walletbackup.py",
- "time": 101
+ "time": 137
},
{
"name": "zapwallettxes.py",