-
Notifications
You must be signed in to change notification settings - Fork 38.9k
mining: always pad scriptSig at low heights, drop include_dummy_extranonce #34860
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
base: master
Are you sure you want to change the base?
Changes from all commits
7881e29
f10d969
51d748c
d6f9ab4
d2af27b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -184,14 +184,18 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock() | |
| // increasing its length would reduce the space they can use and may break | ||
| // existing clients. | ||
| coinbaseTx.vin[0].scriptSig = CScript() << nHeight; | ||
| if (m_options.include_dummy_extranonce) { | ||
| // Set script_sig_prefix here, so IPC mining clients are not affected by | ||
| // the optional scriptSig padding below. They provide their own extraNonce, | ||
| // and in a typical setup a pool name or realistic extraNonce already makes | ||
| // the scriptSig long enough. | ||
| coinbase_tx.script_sig_prefix = coinbaseTx.vin[0].scriptSig; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In commit "mining: always set dummy_extranonce at low heights" (d6f9ab4): At heights ≤ 16, |
||
| if (nHeight <= 16) { | ||
| // For blocks at heights <= 16, the BIP34-encoded height alone is only | ||
| // one byte. Consensus requires coinbase scriptSigs to be at least two | ||
| // bytes long (bad-cb-length), so tests and regtest include a dummy | ||
| // extraNonce (OP_0) | ||
| // bytes long (bad-cb-length), so an OP_0 is always appended at those | ||
| // heights. | ||
| coinbaseTx.vin[0].scriptSig << OP_0; | ||
| } | ||
| coinbase_tx.script_sig_prefix = coinbaseTx.vin[0].scriptSig; | ||
| Assert(nHeight > 0); | ||
| coinbaseTx.nLockTime = static_cast<uint32_t>(nHeight - 1); | ||
| coinbase_tx.lock_time = coinbaseTx.nLockTime; | ||
|
|
@@ -221,7 +225,6 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock() | |
| pblock->nNonce = 0; | ||
|
|
||
| if (m_options.test_block_validity) { | ||
| // if nHeight <= 16, and include_dummy_extranonce=false this will fail due to bad-cb-length. | ||
| if (BlockValidationState state{TestBlockValidity(m_chainstate, *pblock, /*check_pow=*/false, /*check_merkle_root=*/false)}; !state.IsValid()) { | ||
| throw std::runtime_error(strprintf("TestBlockValidity failed: %s", state.ToString())); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In commit "mining: always set dummy_extranonce at low heights" (d6f9ab4):
The comment says "in a typical setup a pool name or realistic extraNonce already makes the scriptSig long enough" but this puts the responsibility entirely on the client at
heights ≤ 16. I tested this by removing the extra nonce inrun_low_height_test()and it causessubmitSolution()to silently return False with no explanation. Should we at minimum document in the code or the IPC interface that atheights ≤ 16the client must provide at least 1 byte of extra nonce?