The problem with including <simplicity/elements/env.h> in interpreter.h is that
now everyone who needs to include interpreter.h also needs access to
Simplicity's header files too.
This commit breaks that dependency chain by using forward declarations. It will
pay signifigant dividends when elements switches to CMake.
GetHash now manually implemented for CBlockHeader. SER_GETHASH removed,
so CProof now always serializes `solution`. CBlockHeader::GetHash copies
from Serialize, but only includes `challenge` of CProof, and no signblock witness
This needs to be tested with liquidv1 chain
The main difference is that there is now explicit simplicity deallocation functions to go with the allocation functions in the API.
There are some minor changes to error message text.
The deserialization code is now automatically generated.
The are some other minor internal changes.
We have the code fragment `txTo.GetHash().begin()`, which takes a
transaction, computes its txid as a uint256, and then saves a pointer to
the internal data of the uint256.
However, in C++, expressions of the form a.b().c() lead to the return
value of `b` being dropped immediately after the call to `c`. This is
fine if `c` is something like `GetHex` which returns a new independently
allocated object with no pointers to its input. It is not fine for
`begin` which returns a pointer into the return value of `GetHash`.
So this fragment returns a dangling pointer, which is later used by the
Simplicity interpreter, leading to UB.
In practice this code appeared to work, possibly because the stack
layout was such that it actually did work ok. Or possibly because we
don't test with enough fidelity to tell that Simplicity's view of the
txid of a transaction was mangled.
In order to ensure that the change of nVersion to a uint32_t in the
previous commit has no effect, rename nVersion to version in this commit
so that reviewers can easily spot if a spot was missed or if there is a
check somewhere whose semantics have changed.
dee89438b8 Abstract out ComputeTapbranchHash (Russell O'Connor)
8e3fc99427 Do not use CScript for tapleaf scripts until the tapleaf version is known (Russell O'Connor)
Pull request description:
While BIP-341 calls the contents of tapleaf a "script", only in the case that the tapleaf version is `0xc0` is this script known to be a tapscript. Otherwise the tapleaf "script" is simply an uninterpreted string of bytes.
This PR corrects the issue where the type `CScript` is used prior to the tapleaf version being known to be a tapscript. This prevents `CScript` methods from erroneously being called on non-tapscript data.
A second commit abstracts out the TapBranch hash computation in the same manner that the TapLeaf computation is already abstracted. These two abstractions ensure that the TapLeaf and TapBranch tagged hashes are always constructed properly.
ACKs for top commit:
ajtowns:
ACK dee89438b8
instagibbs:
ACK dee89438b8
achow101:
ACK dee89438b8
sipa:
ACK dee89438b8
aureleoules:
reACK dee89438b8 - I verified that there is no behavior change.
Tree-SHA512: 4a1d37f3e9a1890e7f5eadcf65562688cc451389581fe6e2da0feb2368708edacdd95392578d8afff05270d88fc61dce732d83d1063d84d12cf47b5f4633ec7e
