mining: getblocktemplate proposal mode validates like submitblock (#663)

CHANGELOG.md

@@ -75,6 +75,23 @@ item below is (or will be) written up in full in the in-development
   installed packages come from — was failing the whole step, and with it every
   canary job.
 
+- **Breaking:** `getblocktemplate` proposal mode validates the proposed block
+  the way `submitblock` does. It ran a separate loop that skipped script
+  verification by its own admission, and with it BIP 68 sequence locks, the
+  block sigop cost and BIP 30, so a miner was told a block would be accepted
+  when it would not (#663).
+- `getblocktemplate` answers `duplicate` / `duplicate-invalid` /
+  `duplicate-inconclusive` for a block the node already knows, as Core does,
+  instead of `inconclusive-not-best-prevblk` (#663).
+- **Breaking:** `getblocktemplate` rejects a `mode` it does not understand with
+  `-8 Invalid mode` rather than silently returning a template, and proposal
+  mode without a string `data` is `-3`, as Core's is (#663).
+- **Breaking:** a block that is both oversized and merkle-broken reports
+  `bad-txnmrklroot`, as Core's `CheckBlock` does — it checks the merkle root
+  before the size limits — and `check_block` applies Core's legacy-sigop
+  ceiling, which fired before any prevout was resolved (#663).
+- `-blockversion` overrides the template's block version on regtest, as Core's
+  `CreateNewBlock` does. It was accepted and ignored (#663).
 - **Breaking:** dust thresholds are Bitcoin Core's. satd charged 68 vbytes to
   spend a witness output where Core charges 67, 107 for P2SH where Core charges
   148, and truncated a fee Core rounds up — so P2WPKH was 297 against Core's

docs/release-notes/0.5.2-pre.md

@@ -886,6 +886,69 @@ accepted member sweeps its dust.
 
 Reachable only through `submitpackage`; nothing on the P2P path can produce it.
 
+### `getblocktemplate` proposal mode told miners the wrong answer (#663)
+
+BIP 22's proposal mode exists so a miner can ask "would you accept this
+block?" before spending hash power on it. Core answers it with
+`TestBlockValidity`, which ends in `ConnectBlock(fJustCheck=true)` against a
+throwaway coins view — the same code path a real submission takes, with the
+write discarded.
+
+satd answered it with a second, hand-written loop that reimplemented a subset
+of the rules. It said so itself, in a comment:
+
+> Script verification is skipped — Core's TestBlockValidity runs full script
+> verification, but proposal mode only needs to detect structural and
+> contextual invalidity.
+
+Along with script verification it had no BIP 68 sequence-lock check, no block
+sigop accounting and no BIP 30 test. A miner proposing a block with any of
+those defects was told it was fine, then had it rejected on submission.
+
+The loop is gone. Proposal mode now calls `connect_block` and drops the
+`StoreBatch` it returns — `connect_block` is pure, it reads the store and hands
+its caller a batch to write — so there is exactly one implementation of the
+rules and no way for the two answers to drift apart. A test asserts that
+proposal mode and `submitblock` return the same verdict, and another that the
+tip, the block index and the UTXO set are untouched after a proposal.
+
+The call holds `accept_lock` for its duration, as Core holds `cs_main`: the tip
+must not move between the "builds on the tip" test and the connect.
+
+Three smaller divergences went with it:
+
+- A block the node already knows is `duplicate`, `duplicate-invalid` or
+  `duplicate-inconclusive` depending on what it decided last time
+  (`rpc/mining.cpp`). satd reported `inconclusive-not-best-prevblk` for all of
+  them, because a block already on the chain does not build on the tip.
+- A `mode` that is present but not a string was read as "not proposal", so a
+  caller asking for something satd did not understand was quietly handed a
+  template. It is `-8 Invalid mode` now, and proposal mode with no string
+  `data` is `-3`, matching Core's `RPC_TYPE_ERROR`.
+- An internal error is `-25`, Core's `RPC_VERIFY_ERROR`, not `-1`.
+
+### `check_block` orders two tests the way Core does (#663)
+
+Core's `CheckBlock` runs `CheckMerkleRoot` **first**, ahead of the size limits,
+because "all potential-corruption validation must be done before we do any
+transaction validation": a peer that sent the wrong transactions for a header
+must not cause that header to be marked invalid. satd tested size first, so a
+block that was both oversized and merkle-broken answered `bad-blk-length` where
+Core answers `bad-txnmrklroot`.
+
+satd also had no equivalent of Core's legacy-sigop ceiling in `CheckBlock` — a
+cheap, context-free gate that fires before any prevout is resolved. satd
+counted sigops only in `connect_block`, where the accurate count needs resolved
+prevouts, so a block over the ceiling that also spent nothing resolvable
+reported `bad-txns-inputs-missingorspent` instead. Both cases are in the
+block-consensus matrix and the live differential against bitcoind.
+
+### `-blockversion` was accepted and ignored (#663)
+
+Core's regtest-only `-blockversion=<n>` stamps a chosen version on the template
+header, which is how `mining_basic.py` sets up forking scenarios. satd listed
+the option among its known configuration keys and then never read it.
+
 ### Dust thresholds are Bitcoin Core's (#661)
 
 `GetDustThreshold` prices an output by what it costs to spend: Bitcoin Core