Improve full & compressed inner node deserialization

[nbougalis]

Details

There are two serialization formats for inner nodes: "full" and "compressed". A full node is serialized as 16 32-byte hashes, for a total of 512 bytes. An inner node is serialized as a number of {position, hash} pairs, between 1 and 16, where position is stored as 1 byte and a hash is stored as 32 bytes.

This change removes a redundant copy by using SerialIter instead of Serializer and makes the buffer size checks more stringent.

No change in functionality is expected.

@HowardHinnant is there any reason to keep SHAMapHash as a class instead of converting it to something like: using SHAMapHash = base_uint<256, SHAMap>, other than then fact that we need to pass such hashes to functions which accept a raw uint256 and we have no easy easy to "bend" the type system enough to cast a base_uint<N, X> to base_uint<N, Y> without copying?

Type

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (non-breaking change that only restructures code)
• Tests (You added tests for code that already exists, or your new feature included in this PR)
• Documentation Updates
• Release

[HowardHinnant]

I find this to be a significant improvement in readability.

The only reason I can see for the separate SHAMapHash class is that it has an API that is a subset of base_uint. That being said, I don't think it would be especially error-prone to make the API of SHAMapHash and base_uint identical using a type alias.

[seelabs]

Nice use of SeriaIter to clean up the code. LGTM, except I get a new warning from gcc that we should resolve. Thumbs up after that's fixed.

[seelabs]

gcc warns here:

warning: comparison is always false due to limited range of data type [-Wtype-limits]

Yes, I know this isn't new code; I'm not sure why it didn't use to warn, but we probably want to fix the warning anyway. (pos is unsigned, so it can't be less than 0).

[nbougalis]

The only reason I can see for the separate SHAMapHash class is that it has an API that is a subset of base_uint. That being said, I don't think it would be especially error-prone to make the API of SHAMapHash and base_uint identical using a type alias.

It's a real pity that we don't have strongly typed alias support in C⁺⁺. I really do think it would help make a LOT of code cleaner, safer and more maintainable.