Concept (N)ACK issues

[maflcko]

Parsing will never get 100% accuracy, unless everyone is forced to use an exact format.

Here is a collection of parsing issues.

Misdetection

• HACK Concept ACK? #26 (comment)
• not a NACK, retract my NACK Switch hardened derivation marker to h bitcoin/bitcoin#26076 (comment), Remove -mempoolfullrbf option bitcoin/bitcoin#26525 (comment)

Workarounds:

• Edit the later comments by this user after their review to not contain ACK, or
• "If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.", or
• Have the user resubmit their review freshly (with only a single and clear (N)ACK)

[maflcko]

An alternative code fix would be to ignore ACK after an ACK hash. However, this may interfere with the stale-ACK detection, so it should only kick in when ACK hash is not stale.

Fixed it in cf46398.

This also means that a ACK hash, followed by NACK may or may not be misdetected as ACK.

[ryanofsky]

Thanks for fixing!

This also means that a ACK hash, followed by NACK may or may not be misdetected as ACK.

This doesn't necessarily sound too bad if the PR changed and it shows up a stale ACK. Maybe worse in some other situations, though, but it is hard to be perfect.

[maflcko]

Added bitcoin/bitcoin#35215 (review)

cc @l0rinc

I'd say that the missing space in the comment should just be added.

But it is also fine to adjust the parser, or ignore this, idk.

[maflcko]

• ConceptACK hash -> full ACK coins: use SipHash-1-3-UJ for CCoinsMap keys bitcoin/bitcoin#35215 (review)

Fixed in 7554887