Update note classes to be polymorphic
Closes#3194. Lays foundation for introduction of Sapling notes through refactoring and creation of a class hierarchy for Sprout notes. This PR updates some tests, but otherwise is a no-op.
Upstream encoding cleanups
Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#7922
- bitcoin/bitcoin#7825
- bitcoin/bitcoin#8317
- bitcoin/bitcoin#9804
- Only the commit that changed `base58.cpp`
Precursor to #3202.
Return result of boost::apply_visitor
Not returning a value at the end of a non-void function is undefined behaviour.
Given that this managed to pass our full test suite, I guess that GCC looks for
un-returned values at the end of a function and uses them as the return value,
if the keyword is missing. Clang OTOH complains, which is how we spotted this:
https://ci.z.cash/#/builders/16/builds/282
Not returning a value at the end of a non-void function is undefined behaviour.
Given that this managed to pass our full test suite, I guess that GCC looks for
un-returned values at the end of a function and uses them as the return value,
if the keyword is missing. Clang OTOH complains, which is how we spotted this:
https://ci.z.cash/#/builders/16/builds/282
Remove now-unshadowed serialization lines that do nothing
Previously we had both nVersion as a class parameter *and* a serialization
argument, and in several inherited serializers the latter was set to the former,
in order to pass the serialized object's version into underlying parsers. #3180
pulled in the upstream changes to clean this up, and in doing so these lines
became no-ops - setting the class parameter to itself. Clang throws warnings on
this, which turn into errors on the MacOS builder.
We can just remove these, because upstream already had done so in earlier PRs,
indicating that they were not being relied on by underlying parsers.
SetString seems to be passing the length of the wrong variable to
memory_cleanse, resulting in the last byte of the temporary buffer not being
securely erased.
Previously we had both nVersion as a class parameter *and* a serialization
argument, and in several inherited serializers the latter was set to the former,
in order to pass the serialized object's version into underlying parsers. #3180
pulled in the upstream changes to clean this up, and in doing so these lines
became no-ops - setting the class parameter to itself. Clang throws warnings on
this, which turn into errors on the MacOS builder.
We can just remove these, because upstream already had done so in earlier PRs,
indicating that they were not being relied on by underlying parsers.
Load Sapling testnet parameters into memory
This PR depends on https://github.com/zcash/librustzcash/pull/4.
The parameters are not loaded into memory if `zcashd` is configured as a mainnet node -- this is something we can change in the weeks before `2.0.0`.
Details of Sapling datatypes will be filled in later; for now, they are treated
as binary blobs.
Includes code cherry-picked from upstream commit:
7030d9eb47254499bba14f1c00abc6bf493efd91
BIP144: Serialization, hashes, relay (sender side)
Upstream serialization improvements
Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#5264
- bitcoin/bitcoin#6914
- bitcoin/bitcoin#6215
- bitcoin/bitcoin#8068
- Only the `COMPACTSIZE` wrapper commit
- bitcoin/bitcoin#8658
- bitcoin/bitcoin#8708
- Only the serializer variadics commit
- bitcoin/bitcoin#9039
- bitcoin/bitcoin#9125
- Only the first two commits (the last two block on other upstream PRs)
Part of #2074.
Homu