Skip to content

Changes for checking script size and returning Result<Address, Error> for p2sh#655

Merged
dr-orlovsky merged 1 commit intorust-bitcoin:masterfrom
vss96:Limit-Script-Size
Sep 16, 2021
Merged

Changes for checking script size and returning Result<Address, Error> for p2sh#655
dr-orlovsky merged 1 commit intorust-bitcoin:masterfrom
vss96:Limit-Script-Size

Conversation

@vss96
Copy link
Copy Markdown

@vss96 vss96 commented Sep 15, 2021

Added changes in the p2sh function to return a result (#650), created an error for ExcessiveScriptSize, added a constant for MAX_SCRIPT_ELEMENT_SIZE and added a test where the script size exceeds 520.

@vss96 vss96 changed the title Changes for checking script size and returning Result<Address, Error> for p2sh #650 Changes for checking script size and returning Result<Address, Error> for p2sh Sep 15, 2021
Copy link
Copy Markdown
Collaborator

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

Copy link
Copy Markdown
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 48c732e

@dr-orlovsky dr-orlovsky added this to the 0.28.0 milestone Sep 16, 2021
Copy link
Copy Markdown
Collaborator

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. Checked against Bitcoin Core: it has the same named constant with the same value: https://github.com/bitcoin/bitcoin/blob/f4e12fd50c23875f4b5f272c94449eb81de43d5d/src/script/script.h#L32-L33

Congrants with the first contribution!

@dr-orlovsky dr-orlovsky merged commit b7f9849 into rust-bitcoin:master Sep 16, 2021
@vss96 vss96 deleted the Limit-Script-Size branch September 20, 2021 17:10
@Kixunil Kixunil added the API break This PR requires a version bump for the next release label Jan 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API break This PR requires a version bump for the next release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants