multi: make all new txes v3 #1857
Conversation
Summary of ChangesHello @bhandras, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request systematically updates the default Bitcoin transaction version to 3 (TRUC) across various components of the system. This foundational change is implemented to leverage the improved security features of BIP 431, such as enhanced fee-bumping and pinning protection, which are critical for the robustness of Taproot Assets. The update is accompanied by comprehensive testing to confirm the new version's adoption and ensure seamless integration with existing data. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly updates the Bitcoin transaction version from 2 to 3 across the codebase, aligning with BIP 431 for TRUC transactions. The change is consistently applied, and the inclusion of new tests to verify the transaction version and ensure backward compatibility with v2 proofs is a great addition. My review includes a couple of suggestions to improve code clarity and maintainability.
| proof := proofs[0] | ||
|
|
||
| require.Equal(t, int32(3), proof.AnchorTx.Version, | ||
| "Post-commit b3562461: all new transactions should be v3") |
There was a problem hiding this comment.
It's great that you've included a commit hash in this assertion message. However, it might be even more helpful to also include a reference to the BIP (BIP 431) that introduces v3 transactions. This would provide more context for future developers.
| } | ||
|
|
||
| txTemplate := wire.NewMsgTx(2) | ||
| // TODO(bhandras) |
There was a problem hiding this comment.
This TODO was added without any context. Could you please clarify what this refers to or remove it if it's not needed?
|
|
||
| txTemplate := wire.NewMsgTx(2) | ||
| // TODO(bhandras) | ||
| txTemplate := wire.NewMsgTx(3) |
There was a problem hiding this comment.
To improve maintainability and avoid the use of magic numbers, it would be beneficial to define a constant for the new transaction version, for example const BitcoinTxVersion3 = 3. This constant could then be used here and in all other places in this PR where the transaction version is set to 3.
| txTemplate := wire.NewMsgTx(3) | |
| txTemplate := wire.NewMsgTx(BitcoinTxVersion3) |
This commit changes all new Bitcoin transactions from version 2 to version 3 (TRUC - Topologically Restricted Until Confirmed) as defined in BIP 431. Version 3 transactions provide better fee-bumping and pinning protection, which is beneficial for Taproot Assets anchor transactions.
Pull Request Test Coverage Report for Build 18785634524Details
💛 - Coveralls |
2 participants
This commit changes all new Bitcoin transactions from version 2 to version 3 (TRUC - Topologically Restricted Until Confirmed) as defined in BIP 431. Version 3 transactions provide better fee-bumping and pinning protection, which is beneficial for Taproot Assets anchor transactions.