I found this useful for for creating test cases and checking validity.
In the event that alternative implementations do not aim for exact
serilization compatibility, this RPC can be used to validate and
re-serialize their output for testing.
This makes global handling forwards-compatible when new fields are
added, and un-breaks any PSET implementations that serialize
PSBT_ELEMENTS_GLOBAL_TX_MODIFIABLE (which Elements does not yet
implement).
While the order of fields is not explicit in the PSBT/PSET specifications,
third parties need to be able to support both with a single implementation.
Keeping the PSBT fields in the same order makes this significantly
easier.
A single 0x00 byte indicates a zero length field; we must skip parsing
that field otherwise the length will be expected to be read again for
the vector that is passed in to revieve the value.
This allows PSBT_ELEMENTS_GLOBAL_SCALAR to be parsed when it is
serialized according to the spec, i.e. both of the following cases
will correctly parse to the same representation:
$cli decodepsbt 'cHNldP8B+wQCAAAAAQIEAgAAAAEEAQABBQEAJ/wEcHNldAABAgMEBQYHCAkKCwwNDg8QERITFBUWFxgZGhscHR4fIAEAAA=='
and
$cli decodepsbt 'cHNldP8B+wQCAAAAAQIEAgAAAAEEAQABBQEAJ/wEcHNldAABAgMEBQYHCAkKCwwNDg8QERITFBUWFxgZGhscHR4fIAAA'
PSBT_ELEMENTS_GLOBAL_SCALAR is the only PSBT/PSET field that contains
key data but no value data and so is the only field that currently hits
this special case.
This causes crash on elements wallet when dealing with transactions that
have explicit values and confidential assets. This creates a somewhat
serious DoS attack as the sender can cause the reciever's wallet to
crash by partially blinding the change output. To make matters worse,
the wallet initially accepts the transaction, but fails while spending
the output.
This is likely caused by a combination of two bugs:
1) The wallet's current behaviour stores the complete transaction of interest
in CWalletTx instead of just Outpoints. Only that the spend time do we
iterate over all outputs, try to unblind them and check which are
isMine. When calling wtx.GetOutputValueOut() or similar calls, we hit this assertion.
While the current behaviour is okay, I think the correct way is move
the IsMine == ISMINE_NO at the start of the loop. We should not do be
any checks on outputs that are not ours. This is used in multiple
places at different parts of the codebase for different RPCs.
2) When dealing with partially blinded trasactions, ComputeBlindingData
correctly sets value = -1, and the cache byte to 1. When getting the
data again with GetBlindingData for explicit value and confidential
asset, we load the precomputed data with value = -1 and assert the
loaded value be the explicit value in the transaction. This is only true
for explicit value and explicit asset.
The changed assertion checks that written value should be same as the
explicit value that was written only when the amounts are valid
(cherry picked from commit 53a75ebbae)
This causes crash on elements wallet when dealing with transactions that
have explicit values and confidential assets. This creates a somewhat
serious DoS attack as the sender can cause the reciever's wallet to
crash by partially blinding the change output. To make matters worse,
the wallet initially accepts the transaction, but fails while spending
the output.
This is likely caused by a combination of two bugs:
1) The wallet's current behaviour stores the complete transaction of interest
in CWalletTx instead of just Outpoints. Only that the spend time do we
iterate over all outputs, try to unblind them and check which are
isMine. When calling wtx.GetOutputValueOut() or similar calls, we hit this assertion.
While the current behaviour is okay, I think the correct way is move
the IsMine == ISMINE_NO at the start of the loop. We should not do be
any checks on outputs that are not ours. This is used in multiple
places at different parts of the codebase for different RPCs.
2) When dealing with partially blinded trasactions, ComputeBlindingData
correctly sets value = -1, and the cache byte to 1. When getting the
data again with GetBlindingData for explicit value and confidential
asset, we load the precomputed data with value = -1 and assert the
loaded value be the explicit value in the transaction. This is only true
for explicit value and explicit asset.
The changed assertion checks that written value should be same as the
explicit value that was written only when the amounts are valid
7286cdc3b5 wallet: fix some fee calculation bugs (Andrew Poelstra)
ada87dc6db Fix RC version to 22.0.0rc1 (Glenn Willen)
ff439d8d50 Another fix to manpages (Glenn Willen)
93ebe3b98a QT: fix title in custom chains (Pablo Greco)
dea9125620 Fixes to manpage generation; regenerate manpages (Glenn Willen)
7180bdcbc9 guix/prelude: Override VERSION with FORCE_VERSION (Carl Dong)
4f11bbf7d3 guix: ignore additioanl failing certvalidator test (fanquake)
1e4053c4f0 qt: Pre-rc3 translations update (Hennadii Stepanov)
4a7dc8b520 qt: Pre-rc2 translations update (Hennadii Stepanov)
7ce47bf53e qt: Pre-rc2 translations update (W. J. van der Laan)
f4820e59f0 Revert "Add parameter for chain name in initial download string" (Glenn Willen)
9fb51a2a20 generate manual pages for 22 release (Glenn Willen)
ca2d72ae8b elements: Fix build by removing newly-added assertion from upstream that doesn't make sense with assets (Glenn Willen)
a4b194c671 Some small improvements to release notes (Pieter Wuille)
9c9695f97f doc: mention bech32m/BIP350 in doc/descriptors.md (Pieter Wuille)
bf4d58f351 doc: Move 22.0 release notes from wiki (W. J. van der Laan)
5514d409a9 doc: Stop nixing `-` in manual pages (W. J. van der Laan)
4df7e88f6f Improve doc/i2p.md regarding I2P router options/versions (Jon Atack)
45898781ec Fix build with Boost 1.77.0 (Rafael Sadowski)
a94c1d5c61 qt: Handle new added plurals in bitcoin_en.ts (Hennadii Stepanov)
a5d5de2f35 doc: Mention the flat directory structure for uploads (Andrew Chow)
0bfa783a00 guix: Don't include directory name in SHA256SUMS (Andrew Chow)
c9d0d0cbe4 test: fix bug in 22686 (S3RK)
1c028b9a51 clientversion: No suffix #if CLIENT_VERSION_IS_RELEASE (Carl Dong)
58deead9b9 test: Test for ApproximateBestSubset edge case with too little fees (Andrew Chow)
a513722516 wallet: Assert that enough was selected to cover the fees (Andrew Chow)
efac8a33fa wallet: Use GetSelectionAmount for target value calculations (Andrew Chow)
13a256fc9b release: Release with separate SHA256SUMS and sig files (Carl Dong)
1e6ee42132 guix-verify: Non-zero exit code when anything fails (Carl Dong)
0608e9958f gui: ensure external signer option remains disabled without signers (Andrew Chow)
61e168957d qt: Fix regression in "Encrypt Wallet" menu item (Hennadii Stepanov)
68f17e6b67 consensus/params: simplify ValidDeployment check to avoid gcc warning (Anthony Towns)
8ec2c4fed5 ci: Run fuzzer task for the master branch only (Hennadii Stepanov)
c0bbec8982 build: Bump RC to rc2 (W. J. van der Laan)
a37b834a6b doc: add info to i2p.md about IBD time and multiple networks (Jon Atack)
50b66c238e contrib, p2p: update I2P hardcoded seeds (Jon Atack)
925571c7dd guix: Ensure EPOCH_SOURCE_DATE does not include GPG information (Andrew Chow)
8f495aa3ef guix: Remove extra \r from all.SHA256SUMS line ending (Andrew Chow)
2d948930e2 guix, doc: Add a note that codesigners need to rebuild after tagging (Andrew Chow)
c576e88cee guix: Allow changing the base manifest in guix-verify (Andrew Chow)
82d8bdb042 guix: Make all.SHA256SUMS rather than codesigned.SHA256SUMS (Andrew Chow)
a32205f9cc script, doc: guix touchups (jonatack)
bd56b60454 Updated Readme, Corrected the codesign typo (h)
96aaed9813 guix/build: Remove vestigial SKIPATTEST.TAG (Carl Dong)
9cc787300b guix/INSTALL: Misc fixups (Carl Dong)
d1be61f836 guix: Silence getent(1) invocation (Carl Dong)
6c7021777c build: Bump version to 22.0.0rc1 (W. J. van der Laan)
Pull request description:
The changes shown here are mostly backports from bitcoin core.
There are several manual changes by me, near the end.
ca2d72a - Removes a new upstream assertion around change-handling in coin selection. I would really like someone familiar with the coin selection algorithm to look at this. As far as I can tell, the assertion either (1) does not apply because we have substantially changed this due to asset handling, or (2) is caught by things we already check. But I'd like that reviewed.
9fb51a2 - Generates updated manual pages.
f4820e5 - Reverts a change to the translation files. To quote `doc/translation_process.md`:
> For general PRs, you shouldn’t include any updates to the translation source files. They will be updated periodically, primarily around pre-releases, allowing time for any new phrases to be translated before public releases. This is also important in avoiding translation related merge conflicts.
If we want to change our policy around how we handle these, we would need to discuss setting up the translation software ourselves. I don't think we should do that; the existing approach will fall back to English when a translation isn't available, and we have not gotten any complaints about that, to my knowledge.
ACKs for top commit:
psgreco:
ACK 7286cdc3b5 local tests look good now
Tree-SHA512: d40ae2aee7a78de82756c34da72d1fcf308c0419acca733801a9fb4b7a8f5c978673f2664303c00078bd7eabc640bc0f954e90b46f659cf1ad869b7e50c36d15
First, this reverts commit ca2d72ae8b to reinstate
an assertion that was added in Bitcoin #22686. It did not compile because our
`change_and_fee` variable is a map rather than number; I changed it to use
`map_change_and_fee.at(policyAsset)` to match the equivalent change 2 lines down
from a5d97b363b (merge of Bitcoin #22008).
Then fix the following bugs:
1. Change the new test in rpc_fundrawtransaction.py to bump the -maxtxfee value,
which we'd otherwise exceed, failing the test and masking actual failures.
(This was just caused by the extreme fee settings of the test combined with
Elements' large transactions.)
2. Change the fee-output size estimation for `tx_noinputs_size` to be 46 rather
than 44 bytes; we forgot that even null surjection/rangeproofs need a 0 byte
when output witnesses are present. This mistake triggered the new assertion.
3. Correct the logic in which change outputs are sometimes dropped even when
they are the only blinded output in a transaction with blinded inputs. This
would cause the new test to fail with `bad-txn-inputs-ne-outputs`; I'm very
surprised that no existing tests hit this.
(I have an existing comment block in this code where I "promise" that I had
a good reason for doing something mysterious related to blinding. I was not
able to reverse-engineer my intention here, though I think it is related to
this, but since I couldn't understand it I just left this block intact and
worked around it.)
4. This then triggered the assertion again since the coin selection code
assumes that sufficiently-small change will always be dropped. If we prevent
this drop we will have under-funded the transaction.
To fix this we add Yet Another Flag `may_need_blinded_dummy` in which we add
extra weight to `tx_noinputs_size` in the case that we're doing a blinded tx
but have no blind destinations. We turn this off after coin selection if it
turns out that we don't have any blinded inputs, though ofc at that point
much of the damage/inefficiency has already been done..
5. Fix some constants in other functional tests which assumed precise fee
calculations; these precise values changed because of fixes (2) and (4).
There is one new FIXME, which is that the "dummy change" value will now be a
zero-valued OP_RETURN but we still put a full-size rangeproof and surjection
proof on it. There is some plausible privacy benefit to this but not much,
and wasting 5000+ bytes rather than the ~65 needed for an exact-value proof
is not worth it. We will fix this in the future when we overhaul the wallet
blinding logic.
Previously, if the builder exported $VERSION in their environment (as
past Gitian-building docs told them to), but their HEAD does not
actually point to v$VERSION, their build outputs will differ from those
of other builders.
This is because the contrib/guix/guix-* scripts only ever act on the
current git worktree, and does not try to check out $VERSION if $VERSION
is set in the environment.
Setting $VERSION only makes the scripts pretend like the current
worktree is $VERSION.
This problem was seen in jonatack's attestation for all.SHA256SUMS,
where only his bitcoin-22.0rc3-osx-signed.dmg differed from everyone
else's.
Here is my deduced sequence of events:
1. Aug 27th: He guix-builds 22.0rc3 and uploads his attestations up to
guix.sigs
2. Aug 30th, sometime after POSIX time 1630310848: he pulls the latest
changes from master in the same worktree where he guix-built 22.0rc3
and ends up at 7be143a960
3. Aug 30th, sometime before POSIX time 1630315907: With his worktree
still on 7be143a960, he guix-codesigns. Normally, this would result
in outputs going in guix-build-7be143a960e2, but he had
VERSION=22.0rc3 in his environment, so the guix-* scripts pretended
like he was building 22.0rc3, and used 22.0rc3's guix-build directory
to locate un-codesigned outputs and dump codesigned ones.
However, our SOURCE_DATE_EPOCH defaults to the POSIX time of HEAD
(7be143a960), which made all timestamps in the resulting codesigned
DMG 1630310848, 7be143a960e2's POSIX timestamp. This differs from the
POSIX timestamp of 22.0rc3, which is 1630348517. Note that the
windows codesigning procedure does not consider SOURCE_DATE_EPOCH.
We resolve this by only allowing VERSION overrides via the FORCE_VERSION
environment variable.
======================================================================
ERROR: test_revocation_mode_soft (tests.test_validate.ValidateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/tmp/guix-build-python-certvalidator-0.1-1.e5bdb4b.drv-0/source/tests/test_validate.py", line 85, in test_revocation_mode_soft
validate_path(context, path)
File "/tmp/guix-build-python-certvalidator-0.1-1.e5bdb4b.drv-0/source/tests/../certvalidator/validate.py", line 50, in validate_path
return _validate_path(validation_context, path)
File "/tmp/guix-build-python-certvalidator-0.1-1.e5bdb4b.drv-0/source/tests/../certvalidator/validate.py", line 358, in _validate_path
raise PathValidationError(pretty_message(
certvalidator.errors.PathValidationError: The path could not be validated because the end-entity certificate expired 2022-01-14 12:00:00Z
Github-Pull: #24057
Rebased-From: 8588591965
This facilitates the following workflow:
1) Obtain an updated psbt with in_witness_utxo and in_utxo_rangeproof
2) Get the blinding key from the input utxo address obtained from input
script pubkey without revealing master blinding key
3) Rewind the proof to obtain blinding factors and implement stateless
blinding
We forgot this for rc1. Thanks to Hebasto for fixing the import script.
Tree-SHA512: 127d0989dabf95867f4542e7f3134ef5d5045418b1411582772d60759e16a5090e30c83bffbbc44b2a496ba830a66bdb0d8ba0d2def43f3462a4f15edf64953a
This reverts commit 5e3019570f. That commit
erroneously made changes to translation files by hand, conflicting with
the automated translation pipeline.
Reverting will facilitate merging in upstream translation changes.
