Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address::from_script to return a Result #1022

Closed
dpc opened this issue May 29, 2022 · 4 comments · Fixed by #1038
Closed

Address::from_script to return a Result #1022

dpc opened this issue May 29, 2022 · 4 comments · Fixed by #1038

Comments

@dpc
Copy link
Contributor

dpc commented May 29, 2022

#1021 (comment)

@sanket1729
Copy link
Member

Added another fail condition to be checked: #1021 (comment)

@Eunoia1729
Copy link
Contributor

Picking this up

@nlanson
Copy link
Contributor

nlanson commented Jun 2, 2022

Added another fail condition to be checked: #1021 (comment)

@sanket1729 Is this check not already performed when script.is_witness_program() is called within Payload::from_script()?

Relevant code:

} else if script.is_witness_program() {
Payload::WitnessProgram {
version: WitnessVersion::from_opcode(opcodes::All::from(script[0])).ok()?,
program: script[2..].to_vec(),
}
} else {

WitnessVersion::from_opcode(ver_opcode).is_ok()
&& push_opbyte >= opcodes::all::OP_PUSHBYTES_2.to_u8()
&& push_opbyte <= opcodes::all::OP_PUSHBYTES_40.to_u8()
// Check that the rest of the script has the correct size
&& script_len - 2 == push_opbyte as usize

@sanket1729
Copy link
Member

sanket1729 commented Jun 2, 2022

@nlanson, you are correct! This is already being done. Sorry for the confusion. But as far as this issue is concerned, we should have a dedicated error variant here.

Eunoia1729 added a commit to Eunoia1729/rust-bitcoin that referenced this issue Jun 7, 2022
Modify from_script functions to return result instead of option so that, in case of errors, there is more
information on what went wrong.

Resolves: rust-bitcoin#1022
Eunoia1729 added a commit to Eunoia1729/rust-bitcoin that referenced this issue Jun 7, 2022
Modify from_script functions to return result instead of option so that, in case of errors, there is more
information on what went wrong.

Resolves: rust-bitcoin#1022
apoelstra added a commit that referenced this issue Jun 8, 2022
66e852c Update format of ExcessiveScriptSize error message (eunoia_1729)
89bd4b6 Modify from_script functions in address.rs to return result (eunoia_1729)

Pull request description:

  Modify from_script functions to return result instead of option so that, in case of errors, there is more
  information on what went wrong.

  Resolves: #1022

ACKs for top commit:
  sanket1729:
    ACK 66e852c. LGTM
  tcharding:
    ACK 66e852c
  apoelstra:
    ACK 66e852c

Tree-SHA512: 0d9529aee0a5459351bed2cc8b2c5571736d3293e2931c43d98f53330e9ac5f3d998a19da2b4575af0a3c1c4dcfd5a24c8813390bf6f5492a689c36ebb9cb426
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants