Skip to content

Commit a33a190

Browse files
domob1812domob1812
committed
Decouple CAuxPow from CBaseMerkleTx.
This refactors CAuxPow to be independent of CBaseMerkleTx. Instead, the fields and serialisation code are put directly in CAuxPow as needed. By doing so, we can also get rid of two redundant fields: The hashBlock (which is always zero anyway since the last commit) and nIndex, which is also always zero for a coinbase tx. Those fields are still present in the serialisation format (for backwards compatibility), but not in the actual instance. Note that this changes the consensus behaviour: Previously, CAuxPow instances with a non-zero nIndex value would be invalid. With this change, the nIndex value is ignored, so that those will be accepted. That is, however, fine, since the auxpow is just used to provide PoW for a block and does not otherwise influence the consensus state. If an attacker makes use of this change to send out blocks that are accepted by new nodes while they are rejected by old nodes, this is equivalent to the attacker simply refusing to send blocks to old nodes (which they can always do if they wish, independent of that particular change). For reference, see namecoin/namecoin-core#288.
1 parent 4d606ae commit a33a190

File tree

6 files changed

+44
-38
lines changed

6 files changed

+44
-38
lines changed

‎src/auxpow.cpp‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,6 @@ bool
4141
CAuxPow::check (const uint256& hashAuxBlock, int nChainId,
4242
const Consensus::Params& params) const
4343
{
44-
if (coinbaseTx.nIndex != 0)
45-
return error("AuxPow is not a generate");
46-
4744
if (params.fStrictChainId && parentBlock.GetChainId () == nChainId)
4845
return error("Aux POW parent has our chain ID");
4946

@@ -57,12 +54,11 @@ CAuxPow::check (const uint256& hashAuxBlock, int nChainId,
5754
std::reverse (vchRootHash.begin (), vchRootHash.end ()); // correct endian
5855

5956
// Check that we are in the parent block merkle tree
60-
if (CheckMerkleBranch(coinbaseTx.GetHash(), coinbaseTx.vMerkleBranch,
61-
coinbaseTx.nIndex)
57+
if (CheckMerkleBranch(coinbaseTx->GetHash(), vMerkleBranch, 0)
6258
!= parentBlock.hashMerkleRoot)
6359
return error("Aux POW merkle root incorrect");
6460

65-
const CScript script = coinbaseTx.tx->vin[0].scriptSig;
61+
const CScript script = coinbaseTx->vin[0].scriptSig;
6662

6763
// Check that the same work is not submitted twice to our chain.
6864
//
@@ -191,11 +187,10 @@ CAuxPow::createAuxPow (const CPureBlockHeader& header)
191187
parent.hashMerkleRoot = BlockMerkleRoot (parent);
192188

193189
/* Construct the auxpow object. */
194-
std::unique_ptr<CAuxPow> auxpow(new CAuxPow (coinbaseRef));
190+
std::unique_ptr<CAuxPow> auxpow(new CAuxPow (std::move (coinbaseRef)));
191+
assert (auxpow->vMerkleBranch.empty ());
195192
assert (auxpow->vChainMerkleBranch.empty ());
196193
auxpow->nChainIndex = 0;
197-
assert (auxpow->coinbaseTx.vMerkleBranch.empty ());
198-
auxpow->coinbaseTx.nIndex = 0;
199194
auxpow->parentBlock = parent;
200195

201196
return auxpow;

‎src/auxpow.h‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -89,19 +89,19 @@ class CBaseMerkleTx
8989

9090
/**
9191
* Data for the merge-mining auxpow. This uses a merkle tx (the parent block's
92-
* coinbase tx) and a manual merkle branch to link the actual Namecoin block
92+
* coinbase tx) and a second merkle branch to link the actual Namecoin block
9393
* header to the parent block header, which is mined to satisfy the PoW.
9494
*/
9595
class CAuxPow
9696
{
9797

9898
private:
9999

100-
/**
101-
* The parent block's coinbase tx, which is used to link the auxpow from
102-
* the tx input to the parent block header.
103-
*/
104-
CBaseMerkleTx coinbaseTx;
100+
/** The parent block's coinbase transaction. */
101+
CTransactionRef coinbaseTx;
102+
103+
/** The Merkle branch of the coinbase tx to the parent block's root. */
104+
std::vector<uint256> vMerkleBranch;
105105

106106
/** The merkle branch connecting the aux block to our coinbase. */
107107
std::vector<uint256> vChainMerkleBranch;
@@ -126,8 +126,8 @@ class CAuxPow
126126
public:
127127

128128
/* Prevent accidental conversion. */
129-
inline explicit CAuxPow (CTransactionRef txIn)
130-
: coinbaseTx (txIn)
129+
inline explicit CAuxPow (CTransactionRef&& txIn)
130+
: coinbaseTx (std::move (txIn))
131131
{}
132132

133133
CAuxPow () = default;
@@ -138,21 +138,23 @@ class CAuxPow
138138
inline void
139139
SerializationOp (Stream& s, Operation ser_action)
140140
{
141-
READWRITE (coinbaseTx);
141+
/* The coinbase Merkle tx' hashBlock field is never actually verified
142+
or used in the code for an auxpow (and never was). The parent block
143+
is known anyway directly, so this is also redundant. By setting the
144+
value to zero (for serialising), we make sure that the format is
145+
backwards compatible but the data can be compressed. */
146+
uint256 hashBlock;
142147

143-
/* The Merkle block hash in an auxpow is never verified or used for
144-
anything, and the block hash is known anyway from the parent block.
145-
Thus we can just as well set the hash to zero, so that it compresses
146-
better (and we exclude any accidental use in the future). */
147-
if (ser_action.ForRead ())
148-
coinbaseTx.hashBlock.SetNull ();
148+
/* The index of the parent coinbase tx is always zero. */
149+
int nIndex = 0;
149150

150-
/* When writing, we should always have a zero hashBlock already. There
151-
is no code path that actually sets the hashBlock to something nonzero
152-
(and if there is, then it is a bug). */
153-
else
154-
assert (coinbaseTx.hashBlock.IsNull ());
151+
/* Data from the coinbase transaction as Merkle tx. */
152+
READWRITE (coinbaseTx);
153+
READWRITE (hashBlock);
154+
READWRITE (vMerkleBranch);
155+
READWRITE (nIndex);
155156

157+
/* Additional data for the auxpow itself. */
156158
READWRITE (vChainMerkleBranch);
157159
READWRITE (nChainIndex);
158160
READWRITE (parentBlock);

‎src/rpc/blockchain.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,17 +96,16 @@ UniValue AuxpowToJSON(const CAuxPow& auxpow)
9696

9797
{
9898
UniValue tx(UniValue::VOBJ);
99-
tx.pushKV("hex", EncodeHexTx(*auxpow.coinbaseTx.tx));
100-
TxToJSON(*auxpow.coinbaseTx.tx, auxpow.parentBlock.GetHash(), tx);
99+
tx.pushKV("hex", EncodeHexTx(*auxpow.coinbaseTx));
100+
TxToJSON(*auxpow.coinbaseTx, auxpow.parentBlock.GetHash(), tx);
101101
result.pushKV("tx", tx);
102102
}
103103

104-
result.pushKV("index", auxpow.coinbaseTx.nIndex);
105104
result.pushKV("chainindex", auxpow.nChainIndex);
106105

107106
{
108107
UniValue branch(UniValue::VARR);
109-
for (const auto& node : auxpow.coinbaseTx.vMerkleBranch)
108+
for (const auto& node : auxpow.vMerkleBranch)
110109
branch.push_back(node.GetHex());
111110
result.pushKV("merklebranch", branch);
112111
}

‎src/test/auxpow_tests.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,11 @@ class CAuxPowForTest : public CAuxPow
5252
public:
5353

5454
explicit inline CAuxPowForTest (CTransactionRef txIn)
55-
: CAuxPow (txIn)
55+
: CAuxPow (std::move (txIn))
5656
{}
5757

5858
using CAuxPow::coinbaseTx;
59+
using CAuxPow::vMerkleBranch;
5960
using CAuxPow::vChainMerkleBranch;
6061
using CAuxPow::nChainIndex;
6162
using CAuxPow::parentBlock;
@@ -195,9 +196,7 @@ CAuxpowBuilder::get (const CTransactionRef tx) const
195196
LOCK(cs_main);
196197

197198
CAuxPowForTest res(tx);
198-
res.coinbaseTx.nIndex = 0;
199-
res.coinbaseTx.vMerkleBranch
200-
= merkle_tests::BlockMerkleBranch (parentBlock, res.coinbaseTx.nIndex);
199+
res.vMerkleBranch = merkle_tests::BlockMerkleBranch (parentBlock, 0);
201200

202201
res.vChainMerkleBranch = auxpowChainMerkleBranch;
203202
res.nChainIndex = auxpowChainIndex;

‎test/functional/auxpow_mining.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,6 @@ def test_common (self, create, submit):
109109
data = self.nodes[1].getblock (auxblock['hash'])
110110
assert 'auxpow' in data
111111
auxJson = data['auxpow']
112-
assert_equal (auxJson['index'], 0)
113112
assert_equal (auxJson['chainindex'], 0)
114113
assert_equal (auxJson['merklebranch'], [])
115114
assert_equal (auxJson['chainmerklebranch'], [])

‎test/functional/auxpow_zerohash.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,18 @@ def run_test (self):
9696
p2pStore.send_blocks_and_test ([blk], node, success=True)
9797
assert_equal (node.getbestblockhash (), blkHash)
9898

99+
self.log.info ("Sending non-zero nIndex auxpow through RPC...")
100+
blk, blkHash = self.createBlock ()
101+
blk.auxpow.nIndex = 42
102+
assert_equal (node.submitblock (blk.serialize ().hex ()), None)
103+
assert_equal (node.getbestblockhash (), blkHash)
104+
105+
self.log.info ("Sending non-zero nIndex auxpow through P2P...")
106+
blk, blkHash = self.createBlock ()
107+
blk.auxpow.nIndex = 42
108+
p2pStore.send_blocks_and_test ([blk], node, success=True)
109+
assert_equal (node.getbestblockhash (), blkHash)
110+
99111
def createBlock (self):
100112
"""
101113
Creates and mines a new block with auxpow.

0 commit comments

Comments
 (0)