pow, validation: report the condition that actually failed - #397
Conversation
|
Amended: short-circuit restored. The first version hoisted The condition is short-circuited again, with the individual terms computed inside the failure branch where the cost is irrelevant. Also checked: 78/78, diagnostic output unchanged. |
97e7ef2 to
57e0f84
Compare
edtubbs
left a comment
There was a problem hiding this comment.
ACK, good to report all conditions on failure. The (const uint8_t*)target == 0 tested the wrong thing and dropping strerror(errno) makes sense since it's not set by these failures.
check_pow() rejects on four conditions but its diagnostic could only ever report two of them honestly. The second column, labelled "target == 0", passed "(const uint8_t*)target == 0" to printf: a NULL test on the pointer init_arith_uint256() had just returned, which is never NULL. It printed 0 unconditionally and never called arith_uint256_is_zero(). The fourth condition, target above the chain's pow_limit, was not printed at all. A rejection caused by either of those two therefore produced output in which every printed field read 0, giving no indication of the cause. This is reachable from the existing test suite: the synthetic cases in test_auxpow_deserialize_e2e() hand the auxpow body to deserialize_dogecoin_auxpow_block() without a preceding 80-byte header parse, so block->header->bits is still zero, set_compact(0) yields a zero target, and check_pow() correctly refuses while appearing to report nothing wrong. Evaluate each condition into a named local, report all four, and include nbits so a zero target is attributable to its source. check_auxpow() separately appended strerror(errno) to four failure messages. Nothing on those paths sets errno; the value is stale or zero, so a consensus rejection routinely printed "Success". Drop it. No functional change: the same inputs are accepted and rejected as before.
57e0f84 to
13d51a5
Compare
Thanks to @edtubbs for flagging the confusing output — the log line he posted is
what prompted this, and it turned out the diagnostic was the problem rather than
the validation.
check_powprinted a rejection reason that could not report two of its own fourfailure conditions. One of the checks was
(const uint8_t*)target == 0, whichtests whether the pointer is null rather than whether the target is zero, so it
was always false. The result was a log line that looked self-contradictory:
validation failing while every reported condition read as fine.
This evaluates each condition into a named local and prints all four, plus
nbits:Which answers Ed's question empirically.
nbits: 0x00000000because thosesynthetic cases hand the AuxPoW body straight to the deserializer with no 80-byte
header parse ahead of it, so
header->bitsis still zero fromdogecoin_auxpow_block_new(). A zeronBitsdecodes to a zero target, and azero target is correctly rejected. Not a regression — expected output from the
2026-06-23 negative-path tests, which were never wrong, only illegible.
Also drops four
strerror(errno)calls fromcheck_auxpow.errnois not set bythese failures, so it printed whatever happened to be left over — the
: Successsuffix on a failure line comes from exactly that.
No functional change. Same code accepted, same code rejected; only the
reporting differs.
78/78.
Note: the same
strerror(errno)pattern appears throughoutblock.c'sdeserialize error paths. Same defect, larger diff, left for a separate change.