Clarity-Bitcoin V5 #21
Merged
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
…ty checks for parse-tx and parse-wtx
|
LGTM |
|
@MarvinJanssen The PR description is good as changelog, we have a change log for deployed version in README.md that will be updated once deployed. Any more ideas for improvements? |
|
Are we going to scope v5 to just the changes made in this PR? I was told by @setzeus that people are waiting for v5 of the library and need to start using it. In that case, are there any feature requests we should still bring into it now? |
|
@MarvinJanssen just the changes in PR for now works* we can push a 5.x version later if any feature is requested. |
feat: remove non-compact version
friedger
approved these changes
Dec 3, 2023
| ;; @version 4 | ||
|
|
||
| ;; version 4 adds support for segwit transactions | ||
| ;; @version 5 |
There was a problem hiding this comment.
A one liner about the changes in v5 should go here as well.
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.
Current additions:
parse-wtxnow returnsERR-NOT-SEGWIT-TRANSACTION((err u12)) if provided with a non-segwit transaction. In v4 it returns an empty transaction tuple.parse-txandparse-wtxno longer allow leftover data. In v4 one could append arbitrary data at the end of the Bitcoin transaction and the library would parse the transaction normally but it would use the leftover data for TXID calculation, leading to the wrong TXID and potential security risks. The functions are now more strict and will returnERR-LEFTOVER-DATA((err u13)) if there is any leftover data.was-tx-minedis removed, does not appear to be used and is inefficient. It also had no wtx counterpart. See feat: remove non-compact version #26.We can do optimisations once we the feature set of V5 is fully scoped.