[backport] Backport #1121 and #1131 to elements-22.x for rc3 #1137
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
QT: fix title in custom chains
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
…_blind Check the value assertion only on valid amounts
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.
Note the re-parsing of the correct serialization is covered by the existing PSBT tests which process PSBTs containing scalars.
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.
ASSET_COMMITMENT and ASSET were in the wrong order, which prevents simply iterating the constants in simple parsers/writers.
As for outputs. ISSUANCE_VALUE_COMMITMENT and ISSUANCE_VALUE were misordered.
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).
This allows these test cases to be re-used by alternate implementations for round-trip serialization testing.
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.
…lind_key Allow dumpblinding key to accept non-CT address
PSET: Various fixes
…lements 22rc3.
|
Compared to locally constructed back port -- only delta is rc version bump. utack a17887b |
|
Compared each commit's diff to their diff on master and confirmed that they match exactly (aside from the rc version bump). utACK a17887b |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport #1121 and #1131 to elements-22.x for rc3.
(Because I ended up just doing this as a merge of master into the release branch, #1130 and #1134 also appear in it, which makes the history confusing -- sorry. Those are no-ops here because the commits were already cherry-picked into the branch.)