cryptonote_basic: fix add_extra_nonce_to_tx_extra() length

[jeffro256]

Serialization/deserialization code in tx_extra.h treats the nonce length as a varint, but cryptonote::add_extra_nonce_to_tx_extra() writes the nonce length as a raw unsigned 8-bit integer. For nonce sizes in $[128, 256)$, these two sections diverge. This commit amends cryptonote::add_extra_nonce_to_tx_extra() to match the serialization code.

Issue identified by the Monero Oxide project and reporters.

[jeffro256]

Unit test failure is unrelated.

[jeffro256]

Fixed test cases up and added tests for tx_extra_merge_mining_tag and add_mm_merkle_root_to_tx_extra().

[selsta]

UB if nonce_size = 0

[jeffro256]

For the record, this probably is fine in practice since operator[]() will return some garbage pointer, and memset() is passed a length of 0, but you're absolutely right that operator[]() when pos < size() is false triggers UB (At least until C++26 with a "hardened implementation", under which scenario it becomes a contract violation, not UB).

Will update

[SChernykh]

n <= 255 maybe?

[SChernykh]

But be careful about type bounds to not make it an infinite loop. So (n <= 255) && n is better, to be sure that it doesn't get stuck.

[SChernykh]

Actually nevermind, I see that you check 255 and 65535 separately further down in the code.

[SChernykh]

n <= 65535

[jeffro256]

Sorry, I made one more change: I added an if branch before the mempy() so that tx_extra[start_pos] isn't called when the nonce is empty (and thus start_pos == tx_extra.size()). This triggers UB when extra_nonce.empty() and actually trips a debug assertion on newer versions of glibc++.