calculate sig ops on fly #597
Conversation
biryukovmaxim
force-pushed
the
sig-op-calculation
branch
from
789e63c to
3e023f7
Compare
biryukovmaxim
force-pushed
the
sig-op-calculation
branch
2 times, most recently
from
75e6d58 to
6e1efd2
Compare
| @@ -94,6 +112,36 @@ fn parse_script<T: VerifiableTransaction, Reused: SigHashReusedValues>( | |||
| script.iter().batching(|it| deserialize_next_opcode(it)) | |||
| } | |||
|
|
|||
| pub fn get_sig_op_count_via_simulation<T: VerifiableTransaction, Reused: SigHashReusedValues>( | |||
There was a problem hiding this comment.
What's the purpose of this function? It's never used
There was a problem hiding this comment.
i was aiming to use it in test, but made them without the function.
the idea is adding some helper function that can derive sig op count that is needed for execution by executing it with TxEngine help
There was a problem hiding this comment.
If the function is only for tests, delete it since it's not used.
If you think it's somehow good for users, add documentation that explains what it does
biryukovmaxim
force-pushed
the
sig-op-calculation
branch
from
b8594c7 to
80eeaa3
Compare
crypto/txscript/src/lib.rs
Outdated
| pub sig_op_limit: u8, | ||
| /// Remaining signature operations that can be executed | ||
| pub sig_op_remaining: u8, |
There was a problem hiding this comment.
Maybe it shouldn't be pub anymore?
crypto/txscript/src/lib.rs
Outdated
| sig_op_remaining.checked_sub(num_sigs).ok_or(TxScriptError::ExceededSigOpLimit(num_sigs as u16, *sig_op_limit))?; | ||
| } | ||
| let num_sigs = | ||
| num_sigs.try_into().map_err(|_| TxScriptError::NumberTooBig("Signatures count mustn't exceed 255".to_string()))?; |
There was a problem hiding this comment.
I had a suspicion that this can create a netsplit, since num_sigs arrived from the stack as i32 so it can theoretically be bigger than 255. But after some reading of the context it seems that this error is actually unreachable, since num_sigs is bounded by num_keys which is bounded by MAX_PUB_KEYS_PER_MUTLTISIG=20.
But just to be sure, can you make a multisig test where the hardfork is inactive and num_sigs=256? I expect to see it fails with Err(TxScriptError::InvalidPubKeyCount(format!("too many pubkeys {num_keys} > {MAX_PUB_KEYS_PER_MUTLTISIG}"))) or Err(TxScriptError::InvalidSignatureCount(format!("more signatures than pubkeys {num_sigs} > {num_keys}")))
There was a problem hiding this comment.
actually it will face first MAX_SCRIPT_ELEMENT_SIZE then MAX_STACK_SIZE and only after that, it will fail with error related to MAX_PUB_KEYS_PER_MUTLTISIG.
Make the fields of RuntimeSigOpCounter private and add: - A constructor (new) that ensures consistency between limit and remaining - Getter methods for accessing the private fields - Updated example code to use the new API This change improves encapsulation by ensuring the counter's internal state can only be modified through the consume_sig_ops method, preventing potential inconsistencies between sig_op_limit and sig_op_remaining that could occur with direct field access.
Migrate to runtime signature operation counting
Changes signature operation counting from static analysis to runtime counting during script execution for improved accuracy.
Key changes
Breaking changes
Todo
Requires manual testing and careful review of sig_op counting logic.