[FlyTheElephant1]

Same as pr #404 except the maturation length is based upon the block hash.

Maturity is attached to the block hash, not the spend. Coins created inside the window keep their assigned lock after RDTS expiry. New coinbases after expiry go back to 100. This is Kyle's note on #402: otherwise an opportunistic miner can work the tail of the window and pay almost nothing.

Depth is batched by (block hash) % 6 so a six-week cliff does not mature on one day:

1/6 of window blocks: 2016 confirmations (~14 days at 10 min)
2/6 of window blocks: 4032 confirmations (~28 days)
1/2 of window blocks: 8064 confirmations (~56 days)
Assignment is by block height. Coinbase transaction size does not change (no extra outputs, DATUM-safe).

#404 is the height based variant of this.

#403 is the MTP-duration variant of #402; this branch stays on block-count maturity like #402 and only changes the schedule and the expiry rule.

Regtest: -extendedcoinbasematurity= requires -rdtsexpiry and the start time must be less than the expiry time.

[FlyTheElephant1]

1cc6d53 Fix to issue that could arise after later code changes suggested by Pleb Miner on discord.

[jasonsopko]

Tested 2ece5d6 on Ubuntu 24.04, gcc, wallet on since the functional test needs it. feature_extended_coinbase_maturity.py passes, and validation_tests, txvalidation_tests, txvalidationcache_tests, mempool_tests, miner_tests and coins_tests are clean.

The residue source is the part I checked hardest, because getting it wrong would be silent rather than loud. GetUint64(0) is the low-order end of the uint256, which on a block hash is the tail of the displayed string and not the zero-padded head, so it is the right word. Over the 2000 most recent mainnet blocks:

residue  GetUint64(0)%6          if it had used the top word
   0       344   17.2 %  (2016)     534   26.7 %
   1       332   16.6 %  (4032)     532   26.6 %
   2       357   17.9 %  (4032)     544   27.2 %
   3       312   15.6 %  (8064)     390   19.5 %
   4       325   16.2 %  (8064)       0    0.0 %
   5       330   16.5 %  (8064)       0    0.0 %

tranche shares: 2016 17.2 % (target 16.7), 4032 34.5 % (33.3), 8064 48.4 % (50.0)

The neighboring word cannot produce residues 4 or 5 at all at this difficulty, so half the schedule would never be assigned and nobody would see an error. It is worth a comment in the code saying which end of the hash this is and why.

The Assert(hash_tip) added in 1cc6d53 is reachable from the fuzz harness.

The commit message says "txmempool check() and the fuzz harness still pass disabled bounds and never hit the Assert". CTxMemPool::check() does pass INT_MAX for both bounds. The coins_view harness does not. It passes two independently fuzzed heights in [0, INT_MAX] and no tip at all:

// src/test/fuzz/coins_view.cpp:259
if (Consensus::CheckTxInputs(transaction, state, coins_view_cache,
        fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()),
        tx_fee_out, CheckTxInputsRules::OutputSizeLimit,
        fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()),
        fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()))) {

hash_tip takes its default of nullptr, and the harness can put a coinbase coin at any height into the cache a few lines up: random_coin comes from ConsumeDeserializable<Coin>, which carries nHeight * 2 + fCoinBase, and lands in the cache through the AddCoin branch at line 67. Any run where the coin's creating height falls between the two fuzzed bounds reaches the Assert.

I did not run the fuzzer for this; I put the same call shape in a unit test, which is enough to show what happens:

./consensus/tx_verify.cpp:222 CheckTxInputs: Assertion `hash_tip' failed.
unknown location(0): fatal error in "check_tx_inputs_null_tip_inside_window": signal: SIGABRT

The test file is in the gist. As it stands the harness will abort instead of fuzzing, so coins_view stops covering everything below that line.

The narrow fix is to pass a tip in the harness. The one I would rather see is dropping the default argument, so that hash_tip = nullptr has to be written out by a caller that means it. The reason the first revision defaulted a missing hash to residue 5 and the second had to Assert is that the signature lets a caller forget, and this one is a consensus entry point.

Two questions rather than claims:

Does anything guarantee that coinbase_height is on the active chain when NodeImpl::coinbaseMaturity runs? It Asserts ActiveChain()[coinbase_height] inside the window, and its caller is CWallet::GetTxBlocksToMaturity, which takes the height from the wallet's own TxStateConfirmed. The wallet learns about disconnects through the validation-interface queue while this reads ActiveChain() live under cs_main. I could not build a case where the two are far enough apart for the lookup to come back null, since an equal-work reorg on this chain does not shorten it, but the safety of an Assert in a wallet path rests on that argument and it is not written down anywhere.

Second, the same lookup returns whatever block is at that height on the active chain, which after a reorg is not the block that created the coin. For a coin that still exists it is the same block, so I think this only ever produces a wrong maturity for a wallet entry that is about to be reorged away. Is that how you read it too?

@Kilombino's point on #404 about CTxMemPool::check() calling CheckTxInputs with INT_MAX bounds applies here unchanged, and here it is load-bearing: it is the reason the Assert is not hit from the mempool self-check. If that call is given real bounds so the self-check mirrors the rule, it will need a tip as well.

nit: validation_tests.cpp picks up a second copy of the consensus/consensus.h, consensus/tx_verify.h, limits and vector includes in the middle of the file, below block_malleation. They are all already at the top.

The calendar-basis finding in my #404 comment applies to this branch unchanged: 2016/4032/8064 are 3.9/7.9/15.8 days at the spacing the chain has actually run, not 14/28/56. So does the incidence question I raised there about which pools' coins these locks land on.

Repro and scripts: https://gist.github.com/jasonsopko/75e2f3e69cefcfa19911378aa4c5a54d

[FlyTheElephant1]

Fix needed for hash generation. Converting to draft proposal until we can validate functionality.

[FlyTheElephant1]

I think I misread jasonsopko. I think he said the code works correctly already. Converting back to review ready proposal.

[jasonsopko]

The distribution on this branch is already uniform. The second column of my hash-residue table was a counterfactual rather than a measurement of your code, and I labeled it badly enough to be read the other way. That one is mine.

CoinbaseHashMod6 uses GetUint64(0), the low-order word. Over the 2000 most recent mainnet blocks, every word:

           ACTUAL (#409)   |         counterfactual, not this branch
 res      GetUint64(0)     |  GetUint64(1)      GetUint64(2)      GetUint64(3)
   0       349   17.4%     |   317   15.8%       315   15.8%       553   27.6%
   1       334   16.7%     |   357   17.9%       351   17.6%       530   26.5%
   2       356   17.8%     |   330   16.5%       344   17.2%       536   26.8%
   3       314   15.7%     |   344   17.2%       333   16.6%       381   19.1%
   4       323   16.1%     |   330   16.5%       334   16.7%         0    0.0%
   5       324   16.2%     |   322   16.1%       323   16.1%         0    0.0%

  GetUint64(0)   chi2 =    4.0   p = 0.55     6/6 residues reachable   <- this branch
  GetUint64(3)   chi2 = 1057.5   p = 0.0000   4/6 residues reachable

Residue 3 at 15.7 % is 1.2 standard deviations below the expected 333.3, and the row as a whole fits uniform at p = 0.55. There is nothing in it to correct. The 0.0 % on residues 4 and 5 belongs to GetUint64(3) alone, the word carrying the leading zeros, which is the word this branch does not use.

So no PR from me here, because the only change it could make is the one that breaks it: moving the residue to the top word puts half the schedule out of reach and reports no error, which is why I checked the source in the first place.

On converting to draft, that is not needed for this. Draft reads as "do not review yet", and an ordinary revision is better served by pushing and saying in a comment what changed. Nothing on this branch is waiting on validation.

I have relabeled the columns in the gist so the counterfactual cannot be read as a measurement, and added residue.py so anyone can reproduce it against their own node: https://gist.github.com/jasonsopko/75e2f3e69cefcfa19911378aa4c5a54d

[FlyTheElephant1]

@jasonsopko I think my last commit addressed concerns from your first comment, and here are the answers to the questions. Let me know if there is anything I missed.

First question: Nothing guarantees the height is still on ActiveChain. A shortening reorg can make ActiveChain()[h] null while the wallet still has that TxStateConfirmed. Assert would abort RPC. The commit returns 100 instead. Display glitch on a coin that is about to be disconnected.

Second question: Yes, that is how I read it. Same-length reorg: ActiveChain()[h] exists but is the new block, not the one that created the wallet's coin. Wrong tranche on a row that is about to be marked conflicted. Consensus does not use this function; ConnectBlock looks up the hash on the chain that contains the coin.

                           | Before (Assert)          | After (return 100)
Consensus / block validity | untouched                | untouched
Normal wallet              | fine                     | fine
Rare reorg miss            | process abort            | one immature/trusted number wrong for a moment

[jasonsopko]

Re-tested on ba78673, same setup as before (Ubuntu 24.04, gcc, wallet on). The wallet path, the include cleanup, and the low-word comment all do what your reply says, and dropping the default argument on hash_tip is the change I asked for. Returning the plain 100 on an active-chain miss is the right call for a display path, and your reading of the same-length reorg case matches mine. feature_extended_coinbase_maturity.py passes and the same six unit suites (validation_tests, txvalidation_tests, txvalidationcache_tests, mempool_tests, miner_tests, coins_tests) report no errors.

The fuzz harness Assert is still there. The commit message says coins_view passes nullptr next to disabled bounds, but the bounds in that call are not disabled. src/test/fuzz/coins_view.cpp:259 still draws ext_start_height and ext_expiry_height from the fuzzer, each anywhere in [0, INT_MAX], and now writes the nullptr out by hand. Any coinbase coin the harness puts in the cache with a height between those two values reaches Assert(hash_tip) at tx_verify.cpp:225. The mempool self-check in txmempool.cpp is the call that passes INT_MAX for both bounds, and that one is fine.

Same unit-test repro as before, updated for the new signature:

./consensus/tx_verify.cpp:225 CheckTxInputs: Assertion `hash_tip' failed.
Running 1 test case...
unknown location(0): fatal error: in "ecm_null_tip_tests/check_tx_inputs_null_tip_inside_window": signal: SIGABRT (application abort requested)
./test/ecm_null_tip_tests.cpp(29): last checkpoint: "check_tx_inputs_null_tip_inside_window" test entry
*** 1 failure is detected in the test module "Bitcoin Core Test Suite"

Last time I said I had not run the harness itself. This time I built it (libfuzzer-nosan preset, clang 18) and ran it three ways. From an empty corpus, 15 minutes and 14.2 million executions found nothing, because the harness first has to produce a transaction whose input matches the one coin in the cache. Seeded with the qa-assets coins_view corpus, it reached the Assert after 1,010,499 executions, about eight minutes:

consensus/tx_verify.cpp:225 CheckTxInputs: Assertion `hash_tip' failed.
==46582== ERROR: libFuzzer: deadly signal
SUMMARY: libFuzzer: deadly signal
stat::number_of_executed_units: 1010499

The third is a 146-byte input built by hand (craft-seed.py in the gist, seed.hex next to it): a coinbase coin at height 500, a transaction spending it, window [0, 4096), no tip. It aborts on every run. Change the expiry to 400 so the coin sits outside the window, two bytes, and the same input runs clean.

The green fuzz jobs on your fork are not evidence either way: test_runner.py replays each qa-assets corpus input once with -runs=1, and that replay is clean here too, 3,542 inputs in two seconds. Those inputs predate the two bound arguments, so they land on the window branch only by accident.

The one-line fix is std::numeric_limits<int>::max() for both bounds in that call, matching CTxMemPool::check(). The window branch then has no fuzz coverage, which the functional test and the unit tests carry. Fuzzing the window for real needs a tip whose ancestors cover every height a fuzzed coin can carry, and that is a bigger change than this PR needs.

Repro test, both inputs, and the logs are in the gist: https://gist.github.com/jasonsopko/75e2f3e69cefcfa19911378aa4c5a54d

[FlyTheElephant1]

Ok should be fixed @jasonsopko . I figured out how to get clang18 onto my bookworm (some reason its not in repo) to reproduce your build. Got the build working but the python seems to be looking for files in a .reorg-watch folder in my user directory now.

[jasonsopko]

Verified at e639c02. The one-line change in coins_view.cpp passes INT_MAX for both bounds next to the explicit nullptr, the same shape as CTxMemPool::check(), so the window branch is unreachable from the harness. Rebuilt the libfuzzer-nosan tree (clang 18) on that head and reran everything from my last comment:

• the hand-built 146-byte seed.bin and the input libFuzzer found on its own both execute clean; on ba78673 both aborted at tx_verify.cpp:225
• corpus replay, 3,542 qa-assets inputs at -runs=1: clean
• seeded fuzz run, 600 s from the same corpus: 1,251,854 executions, no crash, where the run on ba78673 aborted after 1,010,499

That closes the last code-level item from my two comments. The calendar-basis point and the incidence question from #404 still stand, and those are about the schedule rather than the code.

On the .reorg-watch folder: the gist holds material for three separate comments and only two of the scripts want that directory. coinbase-incidence.py and payout-lag.py are the #404/#410 measurements and read ~/.reorg-watch/rewards.json, a reward index that reorg-watch builds from a node's REST interface. They are not part of the harness repro. For that you need only craft-seed.py (or xxd -r -p seed.hex > seed.bin) and then, from the build tree:

FUZZ=coins_view ./build_fuzz_nosan/bin/fuzz seed.bin

residue.py and rdts-end-height.py talk to a node's REST interface on 8332 and need nothing else. There is now a table at the top of the gist README saying which script needs what.

[f4u57x]

Reviewing e639c020722a.

There are some details but what I would say the main flaw is that a hasher can submit only blocks which hash leads to a lower maturity.

Also this constraint on miners revenue, which could lead small miners to join pools with the most steady payouts, they will end up joining the pools who trick the hash based maturity, ending up exactly in the situation we are trying to avoid.

1. XOR mask doesn't cover hash derived maturation selection

   Hash-based assignment is unpredictable before finding a solution, but pools/individuals controlling block publication can discard medium/long-tranche solutions and publish only short-tranche blocks.

   At unchanged difficulty, this requires approximately six valid solutions per published block. An operator with 90% of physical hashrate could still produce 60% of published blocks while filtering this way, increasing the network’s expected short-tranche share from 1/6 to 2/3.

   This also initially slows blocks fourfold. Please evaluate selective publication through difficulty adjustment, including when short-only publication becomes economically preferable. High hashrate permits substantial distribution manipulation; it does not by itself establish profitability.

2. Liquidity and concentration

   Extended maturity delays payment without requiring continued mining. A well-capitalized miner can mine a profitable interval, leave, and collect later. Smaller miners must finance operating costs while rewards remain locked, potentially increasing dependence on pools offering advances.

   Please model a large hashrate departure immediately after a retarget. A 90% departure initially increases expected block spacing tenfold, extending calendar maturity and potentially forcing additional liquidity-constrained miners offline. The question is whether this rule amplifies that feedback more than it deters opportunistic mining.

3. Historical enforcement and overhead

   Expiry ends the creation window, not historical validation requirements. Future validation must still reject historical premature spends.

   ExtendedCoinbaseMaturityBounds repeatedly searches for the same boundaries, including wallet queries under cs_main, and computes expiry even when the deployment is unscheduled. Please consider disabled-deployment and maximum-maturity fast paths, plus caching with correct reorg handling. This is an unmeasured overhead concern, not an established DoS.

   Separately, remove the fork-specific run-ci.yml invocation of FlyTheElephant1/bitcoin/.../ci.yml@29.x-knots with secrets: inherit. If retained, use an approved immutable revision and explicitly limited secrets instead of passing inherited secrets to a mutable external branch.

[FlyTheElephant1]

@f4u57ox Yes they will join pools. This PR will almost guarantee that they will join pools. Datum will do the work of evening out the vesting for the miner so that they get consistent pay according to vesting schedule. The growth in hashrate is already forcing them into using datum anyway, and this PR will accelerate that phenomenon somewhat more, but the datum is maturing and despite knowing this before i started working on 404 that it would have this effect, I still determined that it was an acceptable outcome. Every design has tradeoffs, and that is probably the biggest tradeoff of this design in comparison to the other proposals.

If we did not have datum or another similar technology, I would not have made this proposal in this manner, I would have made it as multiple coinbase outputs for each recipient. In the presence of datum and pools, the multiple outputs thing would make the coinbase transactions enormous. Consequences that you are noting were intentional and selected instead of selecting the consequences of doing it the other way.

1. If they want to only mine short tranche, they discard 5/6ths of their hashrate to do it, and they gain nothing in return.
   If they want to mine only short and medium tranche, they discard half of their hashrate to do it and they gain nothing in return. It is certainly possible that they could imbalance the tranches, but doing so would be extremely self sacrificing so they will not. they would find just as many short tranche blocks whether they sacrifice the mediums or longs or not. those mediums and longs are bonuses even if they think the shorts are better.

   As a thought experiment, you are entirely correct that it is possible. In reality, you are wrong because hashers like money, and if they decide they hate money, that's fine too, if they throw away everyhing except the short tranche if everyone does this, it takes them 6x longer to find blocks, which almost pushes us back to 45 days maturity. (but they wont do this, but if they did it would also hit diff adjustment and drop the diff to adjust for their bizarre behavior then make it 14 days again )

2. I thought about these things and these are part of why I put the short tranche medium and long so that they would be less likely to stall the network than the other proposals suggesting a flat 45 days. The short tranche at current profitability is enough and then some to pay out under current conditions their electrical during the month before their bills come. Your concerns are very valid, and a big part of why I decided that this architectural design was needed.

3. Paragraph 1 and 2: I will take time to process these concerns.
   Paragraph 3: Good idea I'll get that done. I am still struggling to figure out how other people have the CI working and mine doesnt work and is also in the PR and not even running.

[f4u57x]

I agree this is a more dynamic maturation selection, I don't know if it's complexity is needed.

Just some personal notes:

I don't see why this is less likely to stall the network than the flat 45 days (or whatever # of blocks we agree).

I might have got something wrong, but when I say pools I'm including sv1 ones, the growth in hashrate hasn't forced them into using Datum otherwise we wouldn't have this alphapool situation.

5/6 it's the maximum they could waste in theory, never underestimate short term profit seekers.

Blocks competing at the last tip will now get preference towards the low maturity one.

Any pool will even out the payment and vesting for miners, not only DATUM. And you could even argue you will get more evened out vesting payments using the largest pool, which as today is Alpha (and his future variants).