Repository navigation
sv2: Stratum v2 Template Provider - #267
Open
cdonnachie wants to merge 42 commits into
Open
cdonnachie wants to merge 42 commits into
cdonnachie wants to merge 42 commits into
Conversation
First piece of the Stratum V2 Template Provider: the wire format, with no node integration and no sockets yet. Sv2 serializes multibyte integers little-endian exactly as Bitcoin does, so most fields need no formatter at all and U256 is already uint256's layout. Only three things diverge and get one: the 24-bit width, which has no native type; the length-prefixed blobs, whose prefix is a fixed-width integer rather than a CompactSize; and BOOL, of which only the least significant bit is meaningful, so 0x02 reads as false rather than failing to parse. Blob reads are batched through MAX_ALLOC_CHUNK so a three-byte length prefix cannot buy a 16 MB allocation for free. Sequence counts need no equivalent because the count prefix itself bounds them to 65535 elements. The message set is deliberately partial: the Common protocol, Extensions Negotiation, the Template Distribution Protocol, and Avian's own multi-PoW extension. Mining and Job Declaration messages are absent because a Template Provider does not speak them. Three Avian-specific messages are defined here. AvianSelectAlgorithm lets a client choose X16RT or MinotaurX, which stock TDP cannot express. AvianTimeConstraints carries absolute ntime bounds, because the spec gives SubmitSolution a lower bound only while X16RT rederives its hash chain every 128 seconds. Both ride on extension id 0x4001, which is in the spec's experimental range and therefore MUST be replaced by a registry-allocated value before any mainnet release; it is defined once so that swap is a one-line change. The tests pin the wire format byte for byte rather than only round-tripping. A codec that agrees with itself but disagrees with the spec would leave the Template Provider unable to talk to anything else, so the expected hex is the actual contract.
Second piece of the Stratum V2 Template Provider, and the one that carried
the project's main technical risk.
That risk is now retired: Sv2's key exchange is the same v2_ecdh that BIP324
defines. Both use X-only ElligatorSwift ECDH on secp256k1 followed by a
tagged hash whose tag is SHA256("bip324_ellswift_xonly_ecdh"), and both
order the two keys by the caller's role. libsecp256k1 documents that
construction for secp256k1_ellswift_xdh_hash_function_bip324, and
CKey::ComputeBIP324ECDHSecret already applies the role remapping Sv2 asks
for. So it is reused verbatim and this module does no curve arithmetic.
What is not shared with BIP324 is everything above the ECDH. BIP324 derives
keys with its own labels and rekeys with FSChaCha20Poly1305, whereas Noise
mixes a chaining key and a transcript hash and uses a plain AEAD with a
message counter. src/bip324.cpp is therefore the structural model here, not
a component to call. The nonce layout does line up: Sv2 wants four zero
bytes then a little-endian counter, which is what Nonce96{0, counter} is.
Both roles are implemented even though a Template Provider is only ever the
responder, so a test can run a full handshake without a network.
Two deliberate choices worth noting. A failed decryption still advances the
nonce, so a forged frame consumes its counter and cannot be probed
repeatedly. And an absent authority key means the certificate is accepted
unverified, which is trust on first use; it is a real mode, so it is
explicit in the signature rather than implied by a default.
The tests pin the cipher suite name by the hash it produces rather than by
the literal, because the name is hashed into the initial state and one wrong
character would change every transcript while still compiling.
Still outstanding: these tests prove self-consistency, not interoperability.
A transcript generated by another implementation is the only thing that can
confirm that, and it should be added before the extension format is frozen.
Completes the Sv2 transport. The previous change added the handshake and the cipher states; this adds the framing that carries messages over them. The class deliberately knows nothing about sockets: the caller appends what it read, drains what is ready, and writes out what is queued. That is what makes the whole wire format testable without a network, and fuzzable later. Two details drove the design. First, the encrypted header and each payload chunk are separate AEAD operations on one cipher, so they consume consecutive nonces and must be processed strictly in order. A header can therefore not simply be re-read when the rest of its frame arrives, because decrypting it twice would consume a second nonce and desynchronize the stream for good. The decoded header is kept in m_pending instead, and the byte-at-a-time test is what actually exercises that path. Second, a MAC failure latches. An authenticated stream has no safe way to skip a bad frame, so there is nothing to resynchronize to, and a caller that ignores the error must not be able to coax the state machine into reinterpreting the remaining bytes as something else. CiphertextLen is the spec's plaintext-to-ciphertext conversion. The expansion is not a constant: each chunk of at most 65519 bytes gains its own MAC, so one byte past a chunk boundary costs a whole second tag. Verified under ASAN and UBSAN with no sanitizer reports, covering multi-chunk payloads, coalesced frames, byte-at-a-time delivery, tampered headers and bodies, and an out-of-window certificate.
aviand can now accept an encrypted Stratum v2 connection. It serves no templates yet, so this is the session layer only: bind, accept, handshake, SetupConnection, and a clean start and stop. Gated behind WITH_SV2 and off at runtime unless -sv2 is passed, so a default build and a default node behave exactly as before. The listener binds to loopback by default. That is a security decision rather than caution: the Noise NX handshake authenticates this server to the client and not the reverse, so it is not an access control, and an exposed sv2 port is equivalent to an exposed RPC port. -sv2allowip widens it, and is checked at accept before any crypto runs. Only the options this change actually consumes are registered. The template-related ones land with the code that reads them, so that no option exists in the help before it does anything. Two design notes. The Common protocol lives here rather than in the Template Provider, because SetupConnection is what decides whether a session exists at all; everything after it is Template Distribution business and is handed to an EventsInterface. Its default refuses every message, which is the right behaviour for a node that cannot yet serve templates: granting a session and then ignoring requests is worse than closing the connection. Lifecycle follows the torcontrol pattern of free functions over a file-static owner, so node/context.h does not need to learn about sv2 and no include cycle appears. StopSv2 runs before the chainman is torn down, because the Template Provider will later hold BlockTemplate objects that reference it. A rejected connection drains before it closes. Queueing a SetupConnection.Error and then erasing the client immediately, which is what the first version did, closed the socket with the explanation still in the buffer; the client learned only that it had been dropped. Three tests caught that. The tests drive the real listener over loopback sockets, from the handshake through SetupConnection, because the accept path, the protocol state machine and the threading are exactly what cannot be covered without a socket.
Implements sv2-spec extension 0x0001: RequestExtensions and its Success and Error replies, exchanged after SetupConnection and before any Template Distribution message. The machinery is generic over a supported set and a required set rather than hardcoding Avian's extension, which does not exist yet. That keeps this change about the mechanism and lets every policy path be tested now instead of waiting for the extension it will eventually carry. Three outcomes, matching the spec's own examples. A request that names nothing required and at least one extension we have gets Success with the intersection. A request for extensions none of which exist gets Error, and is explicitly not fatal: with nothing required, the client is free to carry on without extensions, so disconnecting would be wrong. A request that omits something required gets Error naming it, with one further attempt allowed before the connection ends. Extension 0x0001 is treated as supported by definition, since it is the negotiation mechanism itself. -sv2extensionpolicy defaults to optional rather than require. Require is the intended end state, but demanding negotiation while the required set is empty would reject stock clients for no benefit at all. The default moves when there is actually something to require. One honest limitation, pinned by a test rather than left implicit: a server cannot announce a required extension unprompted, because RequestExtensions.Error only exists as a response. A client that never negotiates can therefore only be dropped, with no on-wire explanation. That is the cost of the require policy and it belongs in the documentation.
Adds what stock Template Distribution cannot express: which proof-of-work algorithm a connection is for. Extension 0x4001 carries AvianSelectAlgorithm, sent once after extension negotiation and before any template, and the policy default moves to require now that there is finally something worth requiring. The algorithm is immutable for the life of a connection. A connection has one nBits and target stream, so changing it would move the target underneath templates already issued; a second selection is refused with algo-already-selected rather than honoured. version_rolling_mask is the other half of the job. Avian keeps the algorithm in nVersion bits 16-23, and BIP320's mask covers bits 13-28, so it contains them outright. Rolling bit 16 silently switches a share to the other algorithm and the wrong pow limit; rolling bits 17-23 trips bad-algo-id. Whatever an operator configures is therefore filtered rather than trusted, and the default is to permit no rolling at all. A client that obeys the mask it is given cannot change which algorithm its work is judged under. -sv2extensionid exists because 0x4001 is in the spec's experimental range, which production equipment must not use, so this has to become a registry-allocated value before mainnet. It is a development and migration knob only: a runtime override would let a node look like it speaks this protocol while advertising someone else's registered number, so it is rejected on mainnet, and rejected for 0x0000 and 0x0001 whose namespaces already have meanings. Using a feature of an unnegotiated extension is a protocol violation rather than an error reply, since the spec forbids a client from touching anything the server has not confirmed. The wire format of this extension is now what a registry allocation request would freeze: one selection message with its success and error replies, and AvianTimeConstraints already defined for the template changes to send.
The node can now hand a client real work. CoinbaseOutputConstraints tells it how much coinbase room the client needs, and a NewTemplate plus the Avian time constraints come back. Tip-change pushes and SetNewPrevHash follow in a later change, so this is request-driven. Everything NewTemplate carries comes straight out of interfaces::Mining, and most of it needed no work at all. Two parts did. The founder payment falls out for free, which was worth confirming rather than assuming: FillFounderPayment already subtracts it from vout[0] and appends its own output, so coinbase_tx_value_remaining is exactly vout[0] and the required outputs are simply vout[1..]. No second subtraction, and a test asserts value_remaining plus the required outputs equals the subsidy, because getting that wrong would either overpay the miner or leave money unspendable. coinbase_prefix is taken as the first push of the scriptSig rather than by trimming the known trailing OP_0, so it keeps working if the assembler's extranonce placeholder ever changes. The commitment is cross-checked against getCoinbaseCommitment and refused if it is not where getWitnessCommitmentIndex says: serving a template whose commitment a client cannot reproduce yields blocks that fail only at connect time, which is the worst place to find out. ntime bounds are pinned off entirely on chains that allow minimum-difficulty blocks, because nBits itself depends on nTime there, so a rolled timestamp can invalidate the difficulty already issued. Elsewhere the ceiling is the lowest of the X16RT epoch boundary, the consensus future-time limit less a margin, and a configurable forward window. Two things worth noting beyond the message itself. Events callbacks now run with the listener's lock released. The provider legitimately calls back into Connman::Send, which takes that lock, and its own lock sits between the two: dispatching under the lock self-deadlocked, and would also have inverted the lock order against the template thread that is about to start sending unprompted. The deadlock is why the first version of these tests timed out instead of failing. TIME_MASK moves from a define in block.cpp to a constant in block.h. It is the X16RT epoch, and duplicating a consensus value in sv2 to get at it would have been exactly the drift worth avoiding. StartSv2 now refuses a multiprocess mining interface, because the Cap'n Proto schema cannot carry pow_type and every template would silently come back as X16RT.
Finishes what the previous change started. NewTemplate carries no previous block hash of its own, so until now a client had a template it could not build a header from. SetNewPrevHash supplies the prev hash, nBits and the expanded target, and is what actually makes the work usable. A template thread watches for a new tip and reissues work unprompted, because a tip change invalidates every outstanding template and waiting to be asked would leave miners grinding on dead work. The same thread refreshes on an interval, which is what picks up new fees and keeps X16RT templates inside their 128-second epoch; -sv2interval is clamped to 120 seconds so work can never outlive that boundary. waitTipChanged only returns early for a new tip or a node shutdown, neither of which this thread's own interrupt can cause, so it is polled in one-second slices. Waiting out the full interval would otherwise make shutdown hang for up to two minutes. Templates are rebuilt per client rather than shared, because each client reserves a different amount of coinbase room, and a template built for one client's reserved weight is not correct for another's. A tip change clears the cache outright: nothing older can be built on. Two deviations from the plan worth recording. Fee-driven refresh happens on the interval rather than through waitNext's fee_threshold, because that would need a template handle per client and a thread per client to wait on; the practical difference is a bounded delay before a fee rise reaches a miner, so -sv2feedelta is not added. And the target is nBits expanded with no pow-limit clamp, matching what getblocktemplate already reports. The lock ordering fixed in the previous change is what makes this safe: the template thread takes the provider's lock and then the listener's when it sends, and the socket thread no longer holds the listener's lock while calling back, so the two orders cannot meet.
A pool that wants to validate the work it is selling, rather than take the node's word for it, needs the transactions. transaction_list carries them serialized with witnesses, and excess_data carries the coinbase commitment script so the commitment can be rebuilt independently. This is the one message where a few request bytes buy a whole block serialized, encrypted and buffered, so it is metered rather than merely bounded: at most two requests in any ten seconds, checked before any work is done, and a third ends the connection. getBlock copies the entire block, so the serialized list is built on first request and cached, not produced up front for templates nobody asks about. An unknown template id and a stale one are reported differently. Only the newest template survives a refresh, so an id this client was once issued is stale, and one it never saw does not exist; the distinction tells a client whether to wait for new work or stop asking. Neither is a disconnection: the client merely asked for the wrong thing. Two things the tests turned up about this tree rather than about the code. Avian's regtest requires standard transactions, unlike Bitcoin's, so a bare OP_TRUE output is rejected with "scriptpubkey" and the test spends to a P2PKH. And the subsidy is not Bitcoin's, so a test transaction has to take its output amount from the input it spends rather than assume a figure.
Ten methods that take m_mutex themselves were missing the negative capability that clang's -Wthread-safety requires, and AvianSelectAlgorithm rejected through a lambda, whose body hides the enclosing function's lock context so Reject could not be called from it. None of this was visible locally: gcc performs no thread-safety analysis at all, and the fuzz job is the only clang build in CI, which makes it the only place in the project where lock annotations are checked. Every build I had done was gcc, so the first thing to notice was CI. No behaviour changes. The annotations now state what the code already did, which is what makes them worth having: the next person to add a method that takes this lock gets told, rather than finding out from the fuzz job.
The Template Provider suite was living in sv2_connman_tests.cpp so that it could reach the client helpers, which lint-tests.py rightly rejects: a suite in foo_tests.cpp has to be named foo_tests or foo_tests_bar. That was a shortcut taken to avoid extracting the helpers, and the extraction is what was actually needed. The shared client now lives in test/util/sv2.h. Nothing in it asserts: helpers return a value and the caller decides what is fatal, which keeps them usable from both suites and reports a failure at the line that cared. Pulling it out also removed a ConnectTo that had been copied wholesale. Two other fixes the same pass turned up. std::to_string is locale dependent and lint-locale-dependence.py flags it, so the noise tests use strprintf. And reading ActiveChain needs cs_main, which these tests were not taking; harmless while the tests are single threaded, but the annotation is right and clang is the only thing that checks it. The assertion count drops because helper-internal requires became plain returns. The same 78 cases run, and every one of them still passes.
Completes the Template Distribution Protocol: a client that finds a block can now hand it back and have it connected. The ordering of the checks is the security property here, not an optimisation. Avian's proof of work has no cheap partial form the way SHA256d does, so one X16RT or MinotaurX computation costs orders of magnitude more than every field check before it, and a few request bytes must not be able to buy one. CheckSolution is therefore a pure function in its own file, laid out cheapest first: fixed-size fields, then one pass over a bounded buffer, then a handful of SHA256d compressions, and only then the hash. Two things make that ordering testable rather than merely intended. The hasher is injected, so every rejection case in sv2_solution_check_tests runs with one that fails the test if it is called at all. And at the provider level, CBlockHeader::GetHash is the only path into the persistent PowCache, so a submission that leaves PowCacheSize() unchanged demonstrably never had its proof of work computed. The submission path uses ComputePoWHash instead, which touches no cache, meaning a rejected frame also cannot be used to flush a warm one. The checks themselves are stricter than consensus in places, deliberately. Avian enforces the founder payment only in ConnectBlock, so a client that dropped that output would otherwise produce a block we accept at header level and relay before validation refuses it. The same applies to the witness reserved value and to the commitment being the highest-indexed one bearing the magic. Each of those is a rejection here instead. There is no SubmitSolution.Error in the protocol, so a refusal can only be logged and counted; inventing a reply would break a stock client's dispatch. -sv2maxrejects drops a client that keeps failing, since one rolling a field it was told not to will never find a valid block and has no other way to be told.
max_clients_enforced opened a second connection expecting a live socket that the node would then close. Under CI's parallel load the node gets to accept-and-close first, so the client's own connect fails with a reset and the test aborted on a helper's BOOST_REQUIRE. Both outcomes are the refusal working. The claim is that no second client is admitted, not which layer reports it, so accept either.
test/functional/test_framework/sv2.py is a complete Template Distribution client in Python: the Noise NX handshake, the framing, and codecs for every message the node speaks. It is built on the framework's existing BIP324 primitives, because Stratum v2's key agreement is BIP324's v2_ecdh down to the tag and the role-dependent key ordering. Its value is not only coverage. Everything the unit tests assert about the wire format, they assert against the same code that produces it. This is a second reader, written from the spec, so a field in the wrong order or a length prefix of the wrong width now shows up as a decode failure rather than as a round trip that agrees with itself. mining_sv2.py uses it for the end-to-end claim the design is for: a block mined over Stratum v2 by each of Avian's two algorithms, accepted by the node and reported under the algorithm that was negotiated. There is no cheap pre-filter for X16RT or MinotaurX, so the test searches by submitting and watching the tip, which on regtest lands almost immediately because the target is the algorithm's own floor. Two things the test pinned down that are worth recording. The certificate is verified against the authority key read from the datadir rather than trusted on first use, so the signature path is covered end to end. And on regtest and testnet the ntime window is pinned shut rather than merely narrowed, because those chains allow minimum-difficulty blocks, which makes nBits a function of nTime; rolling ntime at all there could produce bad-diff from a template whose nBits was correct when issued. doc/sv2.md is both the operator guide and the specification of the Avian extension, since that is what a third-party pool or miner implements against and what a registry allocation request would freeze. It states plainly that 0x4001 is experimental and must not be used in production, that no unmodified Stratum v2 client can validate an Avian share, that version rolling is unsupported, that there is no cheap pre-filter for Avian's proof of work, and that exposing the port is equivalent to exposing RPC.
The extension was defined only for the link aviand serves, but the Mining Protocol has the same two gaps for the same reasons: a worker has to be told which algorithm its connection carries, and NewMiningJob has a min_ntime with no maximum, so an X16RT miner rolling ntime off the end of a job's epoch computes the wrong hash function there too. AvianSelectAlgorithm already covers the first. It is connection-scoped and carries nothing protocol-specific, so a pool reuses it verbatim. The second needs a new message, because AvianTimeConstraints is keyed on template_id and a worker has no templates. AvianJobTimeConstraints at 0x04 carries the same window keyed on channel and job. A separate message rather than a reinterpretation of the same field, so that an implementation sitting on both links, which is exactly what a pool is, cannot confuse a template id with a job id. This is specified, not implemented. A Template Provider has no occasion to send it; the enumerator is reserved so the number cannot be reused. Writing it down now matters less for the registry allocation, since message numbers inside an allocated extension are ours to assign and adding one later needs no new allocation, than for the people building against this: without it, AvianTimeConstraints reads as the general case when it is the template-side special case, and libavian-sv2 would be designed around the wrong shape. Also records what a pool must get right that the protocol cannot enforce: the target is compared against Avian's proof-of-work hash rather than SHA256d, REQUIRES_VERSION_ROLLING must stay unset to mirror the node's mask of zero, and the pool builds the coinbase so the node's submission checks are the pool's contract, not the worker's. Alongside: the two session flows and the coinbase split are now mermaid, matching doc/design/multiprocess.md, and the node-versus-pool responsibility split is a table, which is what it always was. Restores the executable bit on mining_sv2.py, which a git reset dropped before the previous commit.
The Mining Protocol section said "do not set REQUIRES_VERSION_ROLLING in SetupConnection", without naming the actor. On that link the pool is the upstream, so read as written it told the pool to clear a flag it never sets. The flag is the downstream's request. The upstream's corresponding response flag is REQUIRES_FIXED_VERSION, and the specification makes them mutually exclusive: if the miner set REQUIRES_VERSION_ROLLING, the pool must not set REQUIRES_FIXED_VERSION. So an Avian pool, which cannot honour rolling at all, has to refuse such a miner with SetupConnection.Error rather than answer it and hold it to a fixed version anyway. Stating it as "version rolling is disabled" left that case undefined. Both flag sets are now tabulated with their bit positions and v1 values, which also pins REQUIRES_STANDARD_JOBS. Without that flag a standard channel may be served grouped or extended jobs, and a worker that can only process NewMiningJob has to advertise the limitation. Also walks back the claim that extended channels need nothing further. AvianJobTimeConstraints does apply to them unchanged, since an ntime window does not depend on who assembles the coinbase, but the coinbase side is not a free extension of the standard-channel case: Stratum v2 defines coinbase_tx_prefix and coinbase_tx_suffix without the BIP141 marker, flag and witness data, while Avian's contract requires a specific witness stack and a canonical byte-for-byte reconstruction. Deferred rather than hand-waved, since extended channels are not in this version anyway. Found in review of the pool plan this document is written for.
Two places named the field min_ntime. That was its name in an earlier revision of the specification, between future_job and the current ntime_start, so the text described a protocol that no longer exists. Correcting the name exposed a second, worse error. The spec says of ntime_start that it is the nTime at which hashing starts and that it is not the consensus minimum, so calling AvianJobTimeConstraints.ntime_min a duplicate of it was wrong about more than spelling: ntime_min is a bound on what the node will accept, and a worker reading a start value as a rule is exactly the confusion the extension exists to remove. The two carry the same number and mean different things. Which also means the relationship between them has to be stated rather than assumed. For an active job ntime_min must equal the job's ntime_start; for a future job the bound takes effect when SetNewPrevHash supplies ntime_start, and that value must agree. Both cases matter, because the spec requires the first job after a channel opens to be a future job, so every connection meets the second before the first. The equivalent invariant already holds on the Template Distribution link and mining_sv2.py asserts it, so a pool can rely on the node's two messages agreeing and has only to preserve that as it narrows. Found in review. Verified against 05-Mining-Protocol.md rather than taken on either my memory or the reviewer's word.
The option protected only nVersion bits 16-23. Everything else was openable, and the test that demonstrated it showed the problem: asked for BIP320's conventional 0x1fffe000, the node permitted 0x1f00e000. Bit 28 in there is DEPLOYMENT_TESTDUMMY on every Avian chain. Nothing stopped 0xe0000000 either, which is VERSIONBITS_TOP_MASK, and Avian signals assets inside it with VERSIONBITS_TOP_BITS_ASSETS. So an operator following the obvious BIP320 recipe would have let a client corrupt version signalling. There is no durable safe subset to narrow it to. Bits 16-23 are the algorithm, 29-31 are the top mask, and the remainder is BIP9 deployment space where bits 2, 11, 12 and 28 are assigned and the next deployment takes another, so any mask chosen as safe today stops being safe later. Against that, rolling buys Avian nothing: it exists to give ASICs search space past nonce and extranonce at very high hash rates, and X16RT and MinotaurX are GPU and CPU algorithms where nonce plus extranonce is ample. So version_rolling_mask is now fixed at 0 with no way to change it. The wire field stays and CheckSolution still honours a non-zero value, which is what a later protocol version would use, and a connman test still pins that the field is transmitted faithfully. This also resolves a contradiction the documentation had been carrying, between an option that implied a non-zero mask was possible and a caveat saying version rolling was unsupported. Separately, the future-job rule said constraints could be sent "on activation" without saying on which side of SetNewPrevHash. They must precede it. Otherwise a worker has an instant in which its job is active and its ntime_max is unknown, and the only safe thing it can do in that instant is idle. Stated along with the matching requirement that a worker not start hashing until it holds both its constraints and its activation information. Both found in review of the pool plan.
Specifying that a pool must send AvianJobTimeConstraints before the SetNewPrevHash that activates a job left the node doing the opposite on its own link, with nothing saying why. Read together those look like one of them is a bug. Neither is. A worker activated by SetNewPrevHash is required to start hashing immediately, so constraints arriving afterwards leave a real instant in which it is working without knowing when that work expires. A Template Distribution client is a pool, and it cannot build a single job until it has the whole round in hand anyway, so no such instant exists for it. Sending the constraints last on the node link also buys something. It gives a work round a fixed final message, which a blocking client needs, because SetNewPrevHash is only sent when the tip moved and so cannot distinguish a new-tip round from an in-place refresh. test_framework/sv2.py depends on that, which until now was an undocumented assumption of its read loop; a caller checking that prev_hash is set is also asserting the order, and the docstring now says so.
The provider kept exactly one template per client, so a refresh discarded the previous one and HandleSubmitSolution refused anything built on it with unknown-template. That threw away blocks. -sv2interval defaults to 30 seconds and Avian's nPowTargetSpacing is also 30 seconds, so the refresh cadence equals the block cadence and a pool always has jobs in flight when one lands. Every solution from a job issued before the last refresh was lost, and nothing in the protocol tells the submitter why. It now keeps the last -sv2maxtemplates per client, three by default, which is about ninety seconds of work at the default interval. Templates whose tip has been superseded are still dropped at once, because those genuinely are worthless: the prev hash is wrong, so no solution against them can produce a block that connects. Retention is cheap, which is why the default is not one. A template holds CTransactionRef, so a second one costs a vector of shared pointers rather than another copy of the block. The exception is the serialized transaction list built on demand for RequestTransactionData, which is a real copy of up to a whole block, so PruneTemplates releases it as soon as a template stops being the newest. RequestTransactionData rebuilds it from the handle if a client ever asks again, which it has no reason to do for work it has already been told to replace. Three unit cases: a superseded template is still served, a limit of one restores the old behaviour and reports stale-template, and a new tip drops the old templates outright. The one that matters is in mining_sv2.py, which mines a real block against a template the node has already replaced, since the unit cases only prove the map still holds the entry. That functional test sets both a short interval and a raised retention, for two different reasons: the short interval is how it gets a template superseded promptly, and the raised retention compensates for that same short interval, because the metered nonce search takes tens of seconds and a two second cadence would evict the template where production's thirty second cadence would not.
The initiator's act 1 has four steps and we did three. After MixHash(e.public_key), 04-Protocol-Security.md 4.5.1.1 calls EncryptAndHash over an empty payload, and 4.5.1.2 gives the responder the matching DecryptAndHash. No key exists at that point, so both write nothing to the wire and degrade to MixHash on empty data, which is why message 1 is 64 bytes either way and why HANDSHAKE_MSG1_SIZE was never wrong. It is still part of the transcript. Omitting it left h one SHA256 iteration away from a conforming peer's, and since h is the associated data for message 2, the only symptom was a MAC failure with no diagnostic: the node logged connected and then disconnected and said nothing more. The practical effect was that no specification-conformant Stratum v2 implementation could complete a handshake with aviand. Commit 2 claimed to have retired this risk and retired a different one. What it proved was that Sv2's key agreement is BIP324's v2_ecdh, which is true and is why CKey::ComputeBIP324ECDHSecret is reused verbatim. The transcript above the ECDH was never tested against anything but a second implementation of mine, and test_framework/sv2.py inherited the same omission because it was written from this file rather than from the specification. Both agreed with each other and with nothing else, and every unit case and the functional test pass identically before and after this change, which is the signature of the whole class of bug. Found by an independent client built on the Stratum v2 Reference Implementation, whose noise_sv2 failed against the node with Aead(Error). That client now completes the handshake, and the previous transcript fails against the fixed node, so the node accepts one reading rather than having become permissive. Both sides are written as the specification's own operations rather than an open-coded MixHash, so the transcript reads as the document states it.
An independent implementer working from this document found six places it was not enough to build against. Four are fixed here. AvianSelectAlgorithm.Error was defined as carrying "a STR0_255" and nothing more, so its values were unenumerable from the document even though they are the entire content of the message and part of what a registry allocation freezes. The three the node sends are now tabulated and normative. Two refusal behaviours were undocumented, and both leave a client waiting for something that will not arrive. A second AvianSelectAlgorithm is answered with algo-already-selected and then disconnected, so a client that reads the error and continues gets an unexplained EOF. A malformed extension message gets no reply at all, because replying would require knowing what was meant. That one cost the implementer a debugging round. Also records that under-declaring CoinbaseOutputConstraints is not reported at the time: the coinbase then fails SubmitSolution's canonical re-serialization check, which the protocol gives no way to explain. One naming note added for anyone building on the Stratum v2 Reference Implementation. The implementer reported that the specification calls SetNewPrevHash's third field header_timestamp, which would have made this document wrong. It does not: both 05-Mining-Protocol.md and 07-Template-Distribution-Protocol.md name it ntime_start, which is what this document and the node use. SRI's Rust struct is the thing that diverges, so the note points that out rather than changing anything. The sixth finding, that SetNewPrevHash is documented as conditional while SendNewTemplate sends it unconditionally, is a genuine mismatch between this document and the code and is left for a decision about which should change.
The first implementer of the pool-facing half found five more places this document was not enough to build against. Findings 1 to 6 came from the downstream side; these come from implementing the half Avian Core specifies and does not itself implement, so this is the first time anything has been built against it. SubmitShares.Error has no code for either of the two refusals this extension exists to make possible. A pool must reject a share whose version differs from the job's, because REQUIRES_FIXED_VERSION forbids it and rolling bits 16-23 moves the share to the other algorithm's proof-of-work limit, and one whose ntime falls outside the window. The specification's codes are invalid-channel-id, stale-share, difficulty-too-low and invalid-job-id, none of which describe either. invalid-version and invalid-ntime are now normative here for the same reason the AvianSelectAlgorithm.Error values are: a miner cannot recognise a code it was never told about, and undocumented they sit exactly where the algorithm and the ntime bound sat before this extension existed. Nothing said what a pool does with a worker that clears REQUIRES_STANDARD_JOBS. There is no core code for a flag whose absence is the problem, so this follows missing-declare-tx-data-flag's precedent with missing-standard-jobs-flag rather than straining unsupported-feature-flags, which names flags that are not supported. The ordering rules covered an active job and a future job and not the case a running pool meets most: a channel with an active job on a tip that just moved. It resolves as the future-job case, which keeps the invariant that constraints precede activation. "Narrow, never widen" had no floor. On regtest and testnet the node pins the window shut, so any narrowing at all produces an empty one and a worker with no legal ntime. Where the pool's own coinbase outputs may sit was left to guesswork, and the natural implementation, appending them last, is the one that looks unsafe. It is not: CommitmentIndex takes the highest-indexed output bearing the aa21a9ed magic, which an ordinary payout does not bear, and the required outputs are checked for presence rather than position. So either order works, and the document now says so rather than leaving an implementer to find out by submitting a block.
SendNewTemplate sent all three messages unconditionally, so an in-place refresh carried a SetNewPrevHash announcing a prev hash that had not changed. 07-Template-Distribution-Protocol.md has the message follow the validation of a new best block and describes fee-driven updates as NewTemplate and not SetNewPrevHash, and a future_template of false is defined as meaning the template relates to the SetNewPrevHash already received. So every refresh sent a message saying the previous hash was new, attached to a template declaring it belonged to the one before. The cost fell on the pool. -sv2interval defaults to 30 seconds and nPowTargetSpacing is also 30 seconds, so a pool reading the message's arrival as the tip having moved would discard its in-flight jobs about twice per block, and one reading its absence as the signal would never invalidate at all. Neither reading is unreasonable and both were wrong, which is why the first pool built against this had to compare prev_hash itself. It is not simply a matter of sending it only on a tip change. A client's first template needs one too, even though it is issued with future = false, because that client has not been told any prev hash and has no header to build without it. ClientState therefore remembers the prev hash it has announced, and the message goes out when that changes, which covers both cases with one comparison. doc/sv2.md already described this behaviour, including an argument about AvianTimeConstraints being a work round's terminator that only holds once SetNewPrevHash is conditional. That argument is now true of the code. Tested as behaviour rather than as field values: a refresh must be followed straight by the constraints, GetTemplate asserts the message types it had only named in comments so a first template is covered, and new_tip_reissues_work already covers the tip-change half.
sv2_template_provider_tests/transaction_data_for_a_stale_template failed on CI at the first read after a tip change, having waited the client's ten second timeout for work that never came. It passes locally, including three full runs of the whole suite at -j12, so this is a loaded machine rather than a defect. The two kinds of read are not alike and were sharing a deadline. A reply to a request is produced on the socket thread and arrives promptly or not at all. Pushed work comes from the watcher thread, which polls for a tip change in one second slices and then rebuilds every client's template under cs_main. On a runner executing 160 test binaries four at a time, that chain can take longer than ten seconds without anything being wrong. So PUSH_TIMEOUT is a minute, and the reads that wait on the watcher thread use it. Leaving TIMEOUT alone matters: raising it globally would have made every genuine failure in every suite take six times longer to report, to fix a handful of call sites. Not caused by the SetNewPrevHash change in the previous commit. The failure was on the first message of the round, and that change only decides whether one later message is sent.
A miner that keeps work across a reconnect submits it against whatever job has been given the same id on the new connection. Pools allocate channel and job ids from their own counters and restarting them is the obvious implementation, so this is not a hypothetical: the first pool built against this document hit it, and its own test suite accepted the bad share because regtest's target is the algorithm's floor and almost any nonce passes. At a realistic difficulty the share is merely rejected and the miner has wasted the work and the submission. The hazard is that nothing says so. Neither this extension nor Stratum v2 mentions identifier reuse across connections, and a miner author has no way to infer it from either. The rule is stated on the miner's side, discard work in flight when the connection drops, because that holds whatever the pool does. A pool not reusing job ids is worth doing and is not sufficient. Not added to the registry submission: this is reconnect hygiene for core Stratum v2 rather than anything to do with multiple algorithms or time bounds, so it belongs in a spec clarification rather than in an Avian extension.
Two gaps found by the first solo pool built against this document, plus one I verified while checking its work. The field carries address.workername and nothing documents that. Not Stratum v2, which gives user_identity no structure at all, and not this document. It is convention inherited from Stratum v1 and it is load-bearing: a pool that did not know would parse the whole string as an address and refuse every real miner it met. The split is on the first dot rather than the last, because no Avian address can contain one and address.rig.1 then parses correctly. OpenMiningChannel.Error has no code for an unusable payout address. The specification offers unknown-user and max-count-exceeded and neither fits, yet for a solo pool 'I cannot read where to send the money' is its most important refusal. Four codes are now normative. This is the third time this gap has appeared, after SubmitShares.Error and the missing standard-jobs flag, and the sharpest, because a miner that cannot tell a bad address from a full pool retries forever against one that will never serve it. The verified part: Avian uses base58 version 60 and 122 on mainnet and 111 and 196 on every test network, so mainnet is the only network a base58 address can be checked against. The bech32 prefixes separate regtest and testnet4 but testnet and signet share tavn. The case that matters, a mainnet pool refusing a test address, does work. Not added to the registry submission: a payout address has nothing to do with multiple algorithms or time bounds, so this belongs in Avian's own documentation rather than in the extension, as with finding 12.
The deferral is replaced rather than deleted. It described a conflict that does not exist: a merkle root is folded from txids and a txid excludes witness data, so the witness-free split Stratum v2 asks for is exactly what a miner needs, and Avian's canonical check only ever applies to what the pool submits. Proved against a live node by the first pool to implement it, with a non-empty merkle path, for both algorithms. Three things an implementer needs and could not get anywhere. The scriptSig length byte sits inside coinbase_tx_prefix, before the extranonce bytes it counts, so a pool that does not reserve the whole extranonce there is malformed for every extranonce and sees only shares that never become blocks. The 2-to-100-byte scriptSig is shared three ways once an extranonce joins the height push and an operator tag, and a pool must refuse an extranonce size it cannot fit rather than open a channel whose every coinbase the node rejects, silently, since SubmitSolution has no error reply. And version_rolling_allowed must be false on NewExtendedMiningJob, which REQUIRES_FIXED_VERSION implies without stating for a message carrying its own flag. The last section corrects an overstatement of mine. I justified preferring extended channels by saying a miner can see the transactions it is committing to. It cannot. merkle_path is hashes and only its first element is ever a txid, so a miner can check one named transaction is its immediate sibling and nothing more, which is the whole of a two-transaction block and almost none of a large one. What an extended channel genuinely gives is that the miner assembles the coinbase, so it can confirm its own payout and that it is hashing what it built. For solo mining that is still the question that matters, so the decision stands, but the reason needed saying accurately. Seeing the transaction list is Job Declaration, which is not part of this.
Found by porting cpuminer-opt to this extension, which is the first time anything has read this document as a miner author rather than a pool author. A miner cannot tell whether its own share is a block. It gets a share target and never the network target, because only the pool holds the template and only the pool forwards a winner. Stratum v1's convention is the opposite, compare against the network target and announce locally, so a ported miner goes looking for a field that is not sent and cannot be. Nothing said so. The broader gap is that all 784 lines were written from the node's and the pool's side, while line 5 says the extension is what a third-party pool or miner implements. Everything a miner needs was derivable by inverting the pool-facing prose, which is not the same as being documented. A short miner-side sequence now sits beside the pool-side ones, with the three rules that fail silently if a miner does not hold to them: version exactly as given, ntime inside its inclusive window, and work in flight discarded on a reconnect because job ids are only unique within a connection.
…fixes Found by two reviews of the branch, one of which caught the first of these and one the rest. The mainnet guard is the release blocker. doc/sv2.md, the release notes and the pull request all say 0x4001 must not reach production, and the code enforced that only against -sv2extensionid. The route nobody guarded was the obvious one: aviand -sv2 on mainnet, no arguments, using the compiled-in default. The guard is now on the constant, and it is the check that lifts when an allocation lands. -sv2extensionpolicy=optional could not serve an algorithm to anyone. Under that policy a client reaches READY at SetupConnection, and READY was being read as evidence that an algorithm had already been chosen, so a first AvianSelectAlgorithm was refused as a second. The client.algo_selected flag existed for this and was never read. Only the require policy had tests, which is how it survived. ClientReady now also drops templates cached under a previous algorithm, since it is reachable twice for one client. StopSv2 destroyed the Connman before joining the provider thread that holds a raw pointer to it and calls Send() on it. Everything is stopped before anything is destroyed. The proof-of-work hasher passed consensus.nX16rtTimestamp, which no chain assigns and which is therefore always zero, where chainparams.cpp arms SetPoWHashParams from vUpgrades[UPGRADE_X16RT_SWITCH].nTimestamp. Identical behaviour today because every chain is past that switch, and a latent disagreement between this path and GetHash() about which algorithm a header uses. -sv2port was the one option narrowed by a bare cast rather than checked, so 65536 became 0 and bound an ephemeral port: a node that starts, reports success and listens somewhere nobody was told about. -sv2certvalidity had no ceiling, and not_valid_after is a uint32 of seconds, so a large enough value wraps and every client rejects the certificate as long expired. Each has a test. The optional-policy one asserts a genuine second selection is still refused, so the fix cannot drift into accepting those.
… hash The three denial-of-service findings from review, all of which let one client cost the node more than it should. msg_length is a U24, so the format allows 16 MiB, and the transport honoured that. Nothing a client sends is near it: the largest is SubmitSolution, whose coinbase is a B0_64K. So a client could make the node buffer 16 MiB per connection before the message was dispatched and refused. Inbound payloads are now capped at 256 KiB, rejected on the header rather than after the body is read. max_recv_buffer is also enforced for the first time: it was declared, documented as the per-client cap, and read by nothing, which is worse than having no option at all because it reads as a defence that exists. CoinbaseOutputConstraints was the only unmetered handler and is not cheap: each one rebuilds that client's template under cs_main, and a single full socket read holds about fifteen hundred of these six-byte messages. Metered like the others. The last is the one that was architectural rather than a missing check. ReceivedMessage took the provider-wide lock and held it through HandleSubmitSolution, which ends in a MinotaurX or X16RT hash and then ProcessNewBlock. Rate limiting meant a client could not buy many of those, but each one it did buy stalled every other client and blocked the watcher thread from touching any of them. That is the opposite of the goal the validation ladder exists for. SubmitSolution is now dispatched without the lock and takes it in pieces: one to meter, parse and snapshot an immutable SolutionContext, then nothing held across the hash or the submission, then one to record the outcome. The template handle becomes a shared_ptr so it survives the watcher pruning the template while a hash is in flight, which is the hazard that made the lock look necessary in the first place. Both new limits have tests that attack them. The transport case asserts the byte inside the cap still works, so the limit cannot drift into refusing everything large, and that the refusal happens on the header rather than after the body.
The miner-side rules said to keep ntime inside its inclusive window, which for X16RT is sufficient only because a conforming pool has already folded the epoch end into ntime_max. That fact sat 380 lines earlier under the Template Distribution Protocol, so a miner author reading the miner-side section could not tell whether to clamp independently. Found by the first miner written against this document, whose author wrote a clamp without knowing which it was. It says so now, including that a miner clamping defensively must use the same 128-second rule rather than a wider one, and that MinotaurX must not be clamped as though it had boundaries. Also says what to do when the nonce range is exhausted and the window is pinned shut: wait for the next job. That is the normal case rather than an edge case, because every chain except mainnet sets fPowAllowMinDifficultyBlocks and so pins ntime, and it took reading chainparams.cpp to be sure of it.
The submission allowance exists to stop cheap bytes buying expensive hashes, and it was metering the wrong thing. It counted every submission, so it counted the ones that turned out to be real work, and a pool forwarding blocks as fast as it found them was disconnected by its own node for succeeding. A submission whose proof of work verifies is not what the limit is for. It is work, it cannot be forged, and verifying it was not waste. So its token goes back, before the block is submitted rather than after, because a block that loses a race is still real work and the client should not be charged for having found it. An attacker cannot earn a refund without doing the work the limit exists to protect. Found by the reference pool, which trips this six to sixteen times in every end-to-end run and recovers by reconnecting. Mainnet could not reach it: a pool finding four blocks in ten seconds has other things to think about. But every chain anyone can actually test on sets fPowAllowMinDifficultyBlocks, so the share target is the network target, so every share a miner finds is a block candidate. The limit was therefore wrong everywhere it was reachable and right only where it was unreachable. The functional test respects the distinction it is testing: failed submissions are paced, because those do legitimately consume the allowance, and successful ones are not paced at all, so anything that trips the limit is a success being charged for. The first version of that test did not, and failed on four genuine pow-insufficient results in a row, which is the limit working correctly rather than a regression.
The same treatment SubmitSolution had. HandleRequestTransactionData ran under the provider-wide lock while it copied a block that can be eight megabytes out of the template, serialized every transaction in it, and queued the encrypted result. Every other client's request, every template refresh and every disconnect waited on that. The rate limit made it hard to exploit and did nothing about the serialization it causes when used legitimately. It now snapshots under the lock, builds unlocked, and takes the lock again only to install the result for the next asker. Two requests racing can both build it; throwing one away is cheaper than holding the lock to prevent it. The cached list became a shared_ptr to a const vector so taking a reference to it is a refcount bump rather than another copy of the whole block, which the old code did on every request after the first. A submission that passed every check here and was then refused by validation was reported as SolutionError::NONE, so the log read "refused: ok" for what is the most alarming outcome the system has: these checks and consensus disagreeing. It has its own value now. excess_data carries the witness commitment, which the published specification endorses, and sv2-spec#241 proposes requiring the field empty instead. The question is with the maintainers. The comment says so where the field is set, so this does not become protocol debt nobody remembers choosing. mining_sv2.py now runs in CI. Most of the functional suite is not localized to Avian, which is why the runner was switched off wholesale; this one is, and it is the only test that crosses the Noise handshake, extension negotiation, template delivery and ProcessNewBlock in one go, mining real blocks under both algorithms. A green tick previously did not mean any of that had been exercised. AVIAN_EXTENSION_ID now says why the mainnet guard exists and what would remove it. Refusing mainnet while the identifier is experimental is the conservative reading of a specification that forbids those values in production, not a decision that non-Bitcoin chains must have registered identifiers. That question is open with the registry maintainers, and if the answer is that chains like Avian stay experimental, the guard goes rather than the identifier.
The listener has a single thread and the provider's handlers run on it, so while a submission is being hashed or a block serialized, no other client is being read, written or accepted. Moving the proof-of-work hash out from under the provider mutex removed the contention between clients' provider state; it did not remove this. Bounded rather than solved, and the bounds are the ones already there: both heavy handlers are metered per client before they do any work, a submission whose proof of work verifies is refunded so a pool finding blocks is not charged for it, and -sv2maxclients bounds how many clients exist at all. A pool opens one connection per algorithm, so a node serving a handful of pools is nowhere near this. The structural fix is a worker thread, leaving the listener doing transport only. That is worth doing before a node serves many independent pools and is not worth the concurrency it introduces while it serves a few, so this records the limit and where it bites rather than pre-emptively adding threads to a thing whose contention is currently theoretical. Raised in review of the branch. Noted in the code beside the dispatch that causes it and in doc/sv2.md, because it is a deployment property an operator would otherwise discover by pointing many pools at one node.
Turning the functional runner on in CI exposed a check that had never run. test_runner.py refuses to start if any script in test/functional is absent from its lists, and two were. gen_bip324_avian.py is a generator rather than a test, so it belongs in NON_SCRIPTS beside the other scripts that live there without being run. tool_bitcoin_chainstate.py is deleted. It is byte-identical to tool_avian_chainstate.py apart from the class name, which is the localized version and is the one the runner lists. It was left behind by the rebase onto Bitcoin Core 30.2, nothing references it, and keeping a second copy of a test under a name the project does not use invites editing the wrong one. Verified by running the runner exactly as CI invokes it, which also turned up something worth recording: a build directory configured before a new functional test existed will not have it, because CMake globs that directory at configure time rather than build time. The test then fails instantly with a file-not-found that looks nothing like a stale build. CI configures fresh so it never sees this; a long-lived local build directory does, and the fix is to re-run cmake.
Observed on a live testnet pool, where a miner with a share target far below its hashrate was refused repeatedly while doing nothing wrong. Stratum v2 gives SubmitShares.Error four values and none covers a submission that is valid but too frequent, so a pool enforcing an allowance has to invent one. That makes five places this extension has needed a code the specification does not define, after invalid-version, invalid-ntime, missing-standard-jobs-flag and the payout-address family. The note says which lever answers it, because the obvious one is wrong. A larger allowance buys the same flood at the same cost, since the allowance exists precisely because every share costs a whole X16RT or MinotaurX hash. A harder share target reduces the submissions at source. A miner that sees this should expect its target to change rather than retry.
A miner is given a share target and never the network target, so it cannot tell whether a share it found was a block. For a pool paying by share that is unremarkable, because blocks are the pool's business. For a solo pool it is backwards: the whole proposition is that consensus pays the miner directly for blocks it finds, and the miner is the one piece of software that cannot see them. A live testnet worker had thirty-seven blocks on the pool's page and said nothing about any of them in its own log. The pool holds the answer twice over. It knows a share met the network target, since that is the test deciding whether to forward it. And it can tell whether the block was accepted without asking the node anything: it already hashes every share, so it knows the hash it submitted, and the next SetNewPrevHash carries the new tip. A match means the block connected, which also answers what SubmitSolution leaves open, since that message has no reply and a pool otherwise cannot report more than "sent". Hence two sends rather than a status enum: once when forwarded, once when the tip confirms it. A block that loses a race gets no second message, because the pool cannot distinguish losing from still propagating and claiming either would be worse than silence. Excluded from the registry submission, like the invented error codes and the job-id reuse note before it. Nothing here concerns multiple algorithms or time bounds and any solo pool on any chain has the same gap, so it belongs in core Stratum v2. It lives in this extension because the gap is real now and the specification process is not fast, and an extension allocated for multi-algorithm work should not quietly accumulate unrelated features. This is the sixth thing the extension has needed that Stratum v2 does not define: five error codes and now a message.
CService::IsValid() returns false for the unspecified address, so a bind to every interface was reported as an invalid address. That is a container operator's first instinct, -bind and -rpcbind both take it, and the message did not explain why this one did not. Accept it, and require -sv2allowip alongside it. Connman::IsAllowed would hold the line regardless, since an empty allow list admits loopback only, but it would do so one refused connection at a time with the reason in the node's log rather than in front of whoever started the node. Two things found while fixing it: LookupNumeric reports failure by returning a default-constructed CService, which is sixteen zero bytes tagged IPv6 and so passes IsBindAny(). Accepting the wildcard on that basis would have turned every typo into a wildcard bind, so this now takes the optional- returning Lookup. Lookup discards SplitHostPort's return value, which is why init.cpp checks -bind's port separately. An explicit ":0" therefore arrived intact and bound an ephemeral port: the node came up, reported success, and listened somewhere nobody was told about. That is the hole -sv2port was narrowed to close, reachable through the other option. Also fix a masked assertion in the existing test. ForceSetArg(key, "") sets an empty value rather than unsetting, and an empty value is rejected on its own, so every check after the first in invalid_values_are_rejected was passing on the leftover argument.
doc/sv2.md claimed that adding a message number was free because "an implementation that does not know it will not have negotiated it". That is wrong. Negotiation agrees an extension_type and says nothing about which message numbers inside it either side knows, because the two were built at different times. The first deployment of AvianBlockFound proved it. A correctly encoded 0x05 from an up-to-date pool killed the session of a miner linked against a client that predated the message, and that miner went on hashing and submitting into a connection nobody was serving any more. So state the rule: a receiver MUST ignore a message whose extension_type it negotiated and whose msg_type it does not recognise. Without it, adding a message is not a compatible change at all, it breaks every peer built before it. The node broke the same rule in the other direction. ReceivedMessage disconnected on any extension_type other than core, so a future client-to-server Avian message would have ended an older node's session. Avian's extension now reads past an unrecognised msg_type and logs it. An extension that was never negotiated still disconnects, because that client is not version-skewed, it is confused. An unknown core msg_type still disconnects too: those numbers are fixed by the Stratum v2 specification, not grown by Avian.
Every chain this suite can run on allows minimum-difficulty blocks, which makes nBits a function of the candidate block's own nTime. The template provider therefore pins ntime shut, and the whole window computation never executes: not the -sv2ntimeforward ceiling, not the consensus future-time ceiling, and not the X16RT epoch ceiling that is the reason the window exists at all. Mainnet would have been the first place that code ran. Add -test=no_min_difficulty, regtest-only and following -test=bip94. It turns off the minimum-difficulty rule while fPowNoRetargeting still holds nBits constant, so the chain stays trivially mineable but timestamps stop steering difficulty. The default preserves today's behaviour exactly. The test then asserts what no other chain can show: - MinotaurX gets ntime_max == ntime_min + 600, the -sv2ntimeforward ceiling, a fresh chain being nowhere near the 1290 second consensus limit. - X16RT gets ntime_max == (ntime_min & TIME_MASK) + 127 exactly. The epoch boundary is at most 127 seconds out against a 600 second forward limit, so the epoch is always the binding ceiling and the value is exact rather than bounded. - A block mined at ntime_min + 30 is accepted and the tip carries that timestamp, so nBits really did hold still while it moved. - ntime_max + 1 is still refused without closing the connection. Not asserted: that the X16RT window is non-empty. A template built in the last second of an epoch legitimately has ntime_max == ntime_min, which would be a one-in-128 flake rather than a fault. The MinotaurX case carries that claim instead.
The Sv2 code was written to be fuzzable and then was not fuzzed. solution_check takes its proof-of-work hasher as a parameter for no other reason, and the transport keeps sockets out of its interface for no other reason. These are the first two targets, covering the layers that parse bytes from a peer that has not authenticated anything. sv2_messages decodes arbitrary bytes as each of the eighteen message types and requires that whatever decodes re-serializes to the same length, and for all but one message to the same bytes. NewTemplate is the exception, because BOOL's seven padding bits are ignored on receipt, so 0x02 decodes to false and comes back as 0x00. Whatever normalization happens, happens once: the re-encoding must parse again and produce itself. sv2_messages_roundtrip goes the other way. Random bytes practically never produce a long length-prefixed field, since a 16 MB blob needs three specific prefix bytes followed by 16 MB of anything, so building the message is the only way to reach a 65535-element sequence at its boundary. It also requires the serializer to refuse a field too long for its prefix, that guard being what stops a field being silently truncated on the wire. sv2_transport drives the frame state machine with the fuzzer choosing payload sizes, write sizes and read sizes, and checks that every frame sent arrives once, in order, byte for byte. That covers what a loopback test does not produce reliably: a header decrypted while its body is still in flight, a payload spanning several MAC-tagged chunks, and a partial write. A second mode feeds it garbage, where the oracle is absolute rather than approximate, since a fuzzer cannot forge a MAC and so nothing it writes may ever decode into a message. Two things the harnesses measured about themselves, both of which changed the code rather than being noted and left: The handshake is performed once at startup and copied per run. It takes no fuzzer input, so repeating it bought nothing and cost nearly everything: 62 executions a second with inputs truncated to 8 bytes against 65 with full inputs, which is to say the code under test was five per cent of the work. 212 a second now. It also means no run touches the global random state, so a crash reproduces from its own corpus entry without the framework's zero-seeding. The server identity is leaked rather than destroyed. A CKey allocates from the locked memory pool, and that pool is a static singleton built on the first such allocation, so it finishes construction after a namespace-scope global and is therefore destroyed before one; cleansing key material at that point writes to pages the pool has unmapped. That one was not reasoned about in advance. It arrived as an AddressSanitizer SEGV in memory_cleanse under __run_exit_handlers. Five minutes each under clang with the CI job's sanitizer set: 4.8M, 125K and 27K executions, no findings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this adds
aviandcan serve the Stratum v2 Template Distribution Protocol, so a pool can take block templates over an encrypted Stratum v2 connection instead of pollinggetblocktemplate.It acts as a Template Provider only. Block construction, transaction selection and block submission stay in the node; worker management, share accounting and payouts stay in a separate pool process. That matches the official Stratum v2 architecture and keeps pool-specific code out of the consensus repository.
Nothing is enabled by default. A node started without
-sv2behaves exactly as it does today, and the existing JSON-RPCgetblocktemplate/submitblockpath is untouched.The v5.0 rebase brought in upstream's
interfaces::Mining/interfaces::BlockTemplateabstraction, which exists for exactly this. Template construction, block submission and tip notification needed no changes at all, which is whysrc/sv2/is additive and the rest of the diff is small.Why Avian needs a protocol extension
Stock Template Distribution already carries most of what Avian needs:
NewTemplate.versioncontainspow_type << 16, andSetNewPrevHash.nbitsand.targetare already per-algorithm. Three things are missing, and each is one field:ntime. X16RT derives its hash chain fromSHA256d(nTime & 0xffffff80), so a miner rollingntimepast a 128-second boundary silently computes the wrong hash function.Every one of those failure modes is silent. A miner hits none of them as an error; it just burns hashrate and looks unlucky. That is why the extension is required by default rather than optional.
The extension is small: one message to select an algorithm, one to carry the
ntimewindow, plus success and error replies.doc/sv2.mdspecifies it in full, including the Mining Protocol half that a pool implements.The part that deserves the most review attention
src/sv2/solution_check.cpp, and specifically the ordering of its checks.Avian's proof of work has no cheap partial form the way SHA256d does. There is no way to pre-filter a submission: the only way to know whether it is valid is to compute the whole X16RT or MinotaurX hash, which costs orders of magnitude more than every field check before it. So one attacker frame must not be able to buy one hash.
The ladder is therefore laid out cheapest first, and that ordering is the security property rather than an optimisation:
ntimewindow re-evaluated against the clock, X16RT epoch, coinbase length.Two mechanisms make that ordering proven rather than merely intended:
CheckSolutiontakes the PoW hasher as a parameter, and every rejection case insv2_solution_check_tests.cppinjects one that callsBOOST_FAILif invoked at all. Zero calls is a stronger claim than a counter you might forget to assert.CBlockHeader::GetHash()is the only path into the persistent PowCache, and the submission path deliberately usesComputePoWHash()instead, which touches no cache. So a provider-level test assertingPowCacheSize()does not grow across a rejected submission shows the work was demonstrably never done. It also means rejected frames cannot be used to flush a warm cache.Stricter than consensus, deliberately
Avian enforces the founder payment only in
ConnectBlock, not inContextualCheckBlock. A client that dropped that output would otherwise produce a block the node accepts at header level and relays, only for validation to refuse it later. The same applies to the witness commitment and the BIP141 reserved value. Each is a rejection here instead, which turns a wasted block into a log line.coinbase_tx_value_remainingis already net of the founder payment, so a pool does not subtract it again.Other Avian-specific handling
version_rolling_maskis always 0. There is no durable safe subset ofnVersionto open: bits 16-23 are the algorithm, 29-31 areVERSIONBITS_TOP_MASKwhich Avian also uses to signal assets, and the remainder is BIP9 deployment space where bits 2, 11, 12 and 28 are assigned and the next deployment takes another.ntime_maxis the minimum of the X16RT epoch end,now + MAX_FUTURE_BLOCK_TIME_DUAL_ALGO - margin(Avian's limit is 1350s, not Bitcoin's 7200), andnTime + sv2ntimeforward. On chains allowing minimum-difficulty blocks, which is regtest and testnet, it is pinned tonTimeitself, because therenBitsis a function ofnTimeand any rolling risksbad-diff.-sv2intervaldefaults to 30 seconds andnPowTargetSpacingis also 30 seconds, so the refresh cadence equals the block cadence and a pool always has jobs in flight when a refresh lands. The node retains the last-sv2maxtemplatesper client (3 by default, about 90 seconds of work) so a block found on superseded work is still accepted. Templates for a superseded tip are dropped at once, since those genuinely cannot produce a connecting block.Security
Exposing the Stratum v2 port is equivalent to exposing an RPC port. It binds to loopback by default.
The Noise NX handshake authenticates the server to the client, not the reverse, so it is transport security and not access control.
-sv2allowipis checked at accept time, before any cryptography runs.The cryptography is the spec's
Noise_NX_Secp256k1+EllSwift_ChaChaPoly_SHA256unchanged. Its key agreement turned out to be BIP324'sv2_ecdhdown to the tag and the role-dependent key ordering, soCKey::ComputeBIP324ECDHSecretis reused verbatim andsrc/sv2/noise.cppdoes no elliptic curve arithmetic of its own. Everything above the ECDH is Noise and is implemented here, withsrc/bip324.cppas the structural model rather than a component to call.Testing
Seven unit suites, 104 cases:
sv2_messages_testssv2_noise_testssv2_transport_testssv2_options_testssv2_connman_testssv2_solution_check_testssv2_template_provider_teststest/functional/test_framework/sv2.pyis a complete Template Distribution client in Python, built on the framework's existing BIP324 primitives. Its value is not only coverage: everything the unit tests assert about the wire format, they assert against the code that produces it. This is a second reader, written from the spec, so a field in the wrong order or a length prefix of the wrong width shows up as a decode failure rather than as a round trip that agrees with itself.test/functional/mining_sv2.pyuses it for the end-to-end claim: it mines real regtest blocks over Stratum v2 with each algorithm, confirmsgetblockreports the negotiatedpowtype, verifies the certificate against the authority key read from the datadir, and checks that a block found on a superseded template is still accepted.mining_basic.py,mining_getblocktemplate_longpoll.pyandmining_template_verification.pyare unaffected.Note that CI runs with
RUN_FUNCTIONAL_TESTS=false, somining_sv2.pyis not exercised there. It passes locally.Build
Gated behind
option(WITH_SV2 ... ON), following theWITH_ZMQshape: a plainoption()plus acore_interfacecompile definition, with no external dependency. There is a line in the configure summary.static_assert(MAX_BLOCK_SERIALIZED_SIZE < (1u << 24))inmessages.hmeans a future block-size increase becomes a compile error rather than a silently truncatedmsg_lengthfield.Known limitations, documented rather than papered over
SubmitSolution.Errorin the protocol. A refused solution can only be logged and counted, because inventing a reply would break a stock client's dispatch.-debug=sv2gives the reason, and-sv2maxrejectsdrops a client that keeps failing.RequestExtensions.Errordoes name the required extension, so a client that negotiates the wrong thing can correct itself, but one that never negotiates at all can only be disconnected with a log line and no on-wire explanation.AvianJobTimeConstraintsapplies to them unchanged, but extended-channel coinbase reconstruction is explicitly deferred: Stratum v2 definescoinbase_tx_prefixandcoinbase_tx_suffixwithout the BIP141 marker, flag and witness data, while Avian's contract requires a specific witness stack and canonical byte-for-byte reconstruction.Follow-ups, not in this PR
getsv2infoRPC.solutions_rejected_before_powagainstpow_checksis the observable that proves the DoS ladder is behaving as designed;PowCacheSize()is already in place for itspow_cache_entriesfield.-sv2checkblockoption for optional pre-submit validation.doc/sv2.mdis the frozen wire format it needs.Review notes
The commits are ordered to be read in sequence and each builds and tests on its own. If reviewing by commit, the ones carrying the most risk are
sv2: add the Noise NX handshake and encrypted transportandsv2: validate and accept SubmitSolution.Config surface is in
src/init.cppunderOptionsCategory::SV2; all of it is hidden, anddoc/sv2.mddocuments every option plus the operational and security considerations.