Change Proprietary types to use the method specified in BIP 174 #730
Conversation
|
concept ACK, will review |
|
Really cool! So the proprietary types are already accepted into the BIP, right? |
|
utACK, constant variable name nit. |
|
On a second thought. Are current Elements "PSBT"s compatible with Bitcoin's? Since the tx format is already different from Bitcoin's, an Elements "PSBT" won't be parseable by Bitcoin PSBT logic. Irregardless of using the proprietary keys for out own special fields, shouldn't we update the magic value to something like |
The PR for it is still open IIRC, but I haven't heard any objections.
Perhaps we should, also to make it clear that we're making breaking changes. |
|
Is this missing changes like the compact size wrapper here for non-proprietary fields? bitcoin/bitcoin@6ccda5b#diff-4d99ef1a2a2826595a92ac62c4162790R69 Should we work on upstream merge first to make this simpler? |
Right, perhaps just change the magic, but keep everything else the same to reduce code drift? I have tried to decode an Elements PSBT with Core, and it fails on decoding the global tx.
Yes. I noticed that the changes in bitcoin/bitcoin#17034 were largely incompatible or unrelated to these changes, so I decided not to include them.
Perhaps, but I don't think they are related closely enough. |
If we're making changes to be near-compatible with new bip174, I feel like we should go "all the way" to reduce drift? Maybe I'm misunderstanding. |
It is still compatible, the changes to BIP 174 were designed to be compatible with existing implementations.
Those changes should merge cleanly with the changes (modulo the proprietary types) in Core, so a rebase would be sufficient. It mostly just didn't make sense to me to PR the same commits to both Elements and Core when Elements will just rebase onto Core and pick up those commits later. They aren't particularly important. |
|
fair enough, so |
Elements PSBTs are incompatible with normal PSBTs due to differing transaction formats, so use different magic bytes to distinguish.
achow101
force-pushed
the
bip174-proprietary
branch
from
5a136d8 to
34e1350
Compare
|
Added a commit to use |
|
utACK 34e1350 |
|
LGTM! I'm gonna merge this despite the failure. It's an unrelated test and I don't have the error when run locally. ACK 34e1350 |
…IP 174 34e1350 Use Elements PSBT magic when in Elements mode (Andrew Chow) 3fa355c Change Proprietary types to use the method specified in BIP 174 (Andrew Chow) Pull request description: bitcoin/bips#849 (not yet merged) adds to BIP 174 proprietary types. This PR changes the PSBT types used in Elements to be in line with the proposed changes to the BIP/ Tree-SHA512: 73085a16005448d9f6ad1b74da4a40043ec29738256a439f35106793d11d0e95cb8d728db5cfefe1ddd49cd80367f7f5790d09efaa95bc443e1d6ba295240c4d
…ed in BIP 174 f72e57a Use Elements PSBT magic when in Elements mode (Andrew Chow) 8be2a9e Change Proprietary types to use the method specified in BIP 174 (Andrew Chow) Pull request description: Backport of #730 Tree-SHA512: d0484639049de772989f4f97cc926fbdea0e5c27a294c1f2b5ec0b0f28ca6e620ac5a6306d029ff90d77c1c390848fbad94b3b51fe5e67d4792bb117b000c07b
3 participants
bitcoin/bips#849 (not yet merged) adds to BIP 174 proprietary types. This PR changes the PSBT types used in Elements to be in line with the proposed changes to the BIP/