Fix weight estimation for P2TR inputs #880
Conversation
Estimation can only occur if the P2TR input is a key spend, since we can't reliably predict the size of potential unlocking scripts
| }, | ||
| P2tr => Ok(InputWeightPrediction::P2TR_KEY_DEFAULT_SIGHASH), | ||
| P2tr => { | ||
| // Script spends can't be predicted, so we don't support those |
There was a problem hiding this comment.
We should be able to use the same strategy as #862 at least for the sender where final_script_witness should be provided for all inputs.
There was a problem hiding this comment.
One strategy is to first look at whats on the witness stack. We have two cases:
- You have one element per bip341 we assume its a key spend. If a non-default sighash is used then we need to account for an extra witness byte.
- greater than one witness elements. In which case you can do what we did in Derive weight from witness for p2wsh inputs #862.
If witness is not provided then we can't know how much the input will weight. We need to rely on the application to provide this information (#772).
There was a problem hiding this comment.
We should be able to use the same strategy as #862 at least for the sender where final_script_witness should be provided for all inputs.
IIUC, this would be for a finalized psbt, and the checks i currently have here would be for unfinalized psbts. Would we want to support both finalized and unfinalized psbts? If so, I should keep the existing checks and add the checks for final_script_witness, right?
There was a problem hiding this comment.
Would we want to support both finalized and unfinalized psbts? If so, I should keep the existing checks and add the checks for final_script_witness, right?
yes, exactly. We want to support finalized and unfinalized txs. For the final_script_witness & input.segwit checks we can probably extract what exists for p2wsh and create a common method.
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { |
There was a problem hiding this comment.
The other input pair test live in /src/core/psbt.rs. Any reason these new tests aren't added there?
There was a problem hiding this comment.
oh, i didn't see. I just had them here since that's where the method that's being tested is located, but i can move it
|
Paging @shinghim are you still planning on working on this? |
|
@arminsabouri sorry, got carried away with some other stuff, feel free to work on this |
2 participants
Estimation can only occur if the P2TR input is a key spend, since we can't reliably predict the size of potential unlocking scripts
Closes #788