search

rust-bitcoin / rust-miniscript

Support for Miniscript and Output Descriptors for rust-bitcoin
Creative Commons Zero v1.0 Universal
354 stars 139 forks source link

Planning module: Missing varint length in size calculation of some placeholders #771

Open jp1ac4 opened 1 week ago

jp1ac4 commented 1 week ago

As mentioned in https://github.com/rust-bitcoin/rust-miniscript/pull/481#discussion_r1028718255, it appears that the planning module doesn't include the varint length for TapScript and TapControlBlock when getting the size, but does for other placeholders:

https://github.com/rust-bitcoin/rust-miniscript/blob/acbd120009ce5bee89eedcaafa232ec06a9512b3/src/util.rs#L36-L37

Should the varint length be included here for consistency with other placeholders?

Please also see https://github.com/rust-bitcoin/rust-miniscript/issues/701 for some other inconsistencies.

apoelstra commented 1 week ago

Yeah, I think the varints should always be included since they're part of the witness cost.