--- Log opened Thu Jan 04 00:00:21 2024 04:01 -!- angusp [9e8eed9774@2604:bf00:561:2000::1048] has quit [Write error: Connection reset by peer] 04:01 -!- angusp [9e8eed9774@2604:bf00:561:2000::1048] has joined #bitcoin-rust 04:40 -!- Guest7282 [~Guest7282@user/nex8192] has left #bitcoin-rust [Error from remote client] 04:45 -!- darosior [~darosior@109.205.214.46] has joined #bitcoin-rust 05:02 < darosior> The change to use segwit serialization for transactions with no input is quite surprising and could have been mentioned in the release notes (from https://github.com/rust-bitcoin/rust-bitcoin/compare/bitcoin-0.30.2..bitcoin-0.31.0#diff-4ea069a979edd812a3c3e8b396aa75d9888482207d2acbc84b689a28137a9237L956-L960 to 05:02 < darosior> https://github.com/rust-bitcoin/rust-bitcoin/compare/bitcoin-0.30.2..bitcoin-0.31.0#diff-4ea069a979edd812a3c3e8b396aa75d9888482207d2acbc84b689a28137a9237R968-R978). I'm reading these notes https://github.com/rust-bitcoin/rust-bitcoin/blob/master/bitcoin/CHANGELOG.md#0311---2023-10-18. 05:02 < darosior> The slight change in transaction size estimation it introduced could have gone unnoticed (and maybe it did for some downstream users). I only noticed it because i have unit tests asserting fixed values to "test the unlikely scenario size calculations are modified from under us by upstream". I assume implementation of protocols which rely on being 05:02 < darosior> able to re-construct exactly the same transaction (like Lightning) would test this, but still. 05:02 < darosior> I think this choice complicates working with transactions size. For instance in coin selection, you'd add coins from a pool of coins which aren't necessarily all segwit coins. For the first segwit coin you add, you account for the segwit marker in the transaction size. Now you need to do the opposite: if no segwit input was selected you need to 05:02 < darosior> remember the upstream library actually accounted for those silently and substract 2 weight units to the transaction size. Which is less natural and (very) slightly less accurate. And it's not theoretical that's exactly how the BDK coin selection algorithm is implemented: 05:02 < darosior> https://github.com/bitcoindevkit/coin-select/blob/f147ebcec4b1772874d4314645bc130b38451b94/src/coin_selector.rs#L202-L206. 05:02 < darosior> So that's a cons to choosing this serialization. What were the pros? And what motivated changing the behaviour at all? 05:17 < darosior> The "(very) slightly incorrectly" part regarding the coin selection in my message above is incorrect, because it's part of the "base tx weight" it can just be substracted before performing the selection. It's still very ugly to have to do `empty_tx.weight() - 2` with a long comment explaining the situation when performing coin selection. 05:21 -!- Guest7282 [~Guest7282@user/nex8192] has joined #bitcoin-rust 06:24 -!- Guest7282 [~Guest7282@user/nex8192] has left #bitcoin-rust [Error from remote client] 08:01 < andytoshi> darosior: there was no change to the segwit serialization for many years 08:02 < andytoshi> we may have changed our size estimation functions, in which case it's good that you have fixed test vectors and possibly we need to fix something 08:06 < andytoshi> looks like we made this change in aug 2018 https://www.github.com/rust-bitcoin/rust-bitcoin/pull/153 08:08 < andytoshi> it was released in 0.15.0 and it does appear in the changelog. 08:10 < andytoshi> you can see the linked issue and the docs for the transaction module and type, and the recent issues 2238 and 2295 08:10 < andytoshi> i don't know how we can document this any more clearly :( but patches are welcome 08:10 -!- Guest7282 [~Guest7282@user/nex8192] has joined #bitcoin-rust 08:19 < darosior> andytoshi: no, see the diff links i my first message, there was a change from how `scaled_size()` worked to how the new `weight()` works 08:19 < darosior> Ha these links seem broken.. 08:20 < darosior> Alright let me give you proper link from the 0.30.2 tree vs the 0.31.0 tree 08:23 < darosior> In 0.30.2, `Transaction::weight()` would not account for the segwit marker for transactions with no input: https://github.com/rust-bitcoin/rust-bitcoin/blob/4b6b55f0cd757690c0a815bd0b79d6c7d9508be1/bitcoin/src/blockdata/transaction.rs#L864 08:23 < darosior> https://github.com/rust-bitcoin/rust-bitcoin/blob/4b6b55f0cd757690c0a815bd0b79d6c7d9508be1/bitcoin/src/blockdata/transaction.rs#L956-L960 08:23 < darosior> In 0.31.0, `Transaction::weight()` does account for those: https://github.com/rust-bitcoin/rust-bitcoin/blob/60318c4c7133c4b9df1c7c8c40510bb7e433b1b8/bitcoin/src/blockdata/transaction.rs#L975-L978 08:31 < andytoshi> so, this change does not affect anything about the actual transaction. only the size estimation. and it causes us to correctly estimate the weight (whatever that might mean for an invalid tx) where before we were incorrectly underestimating it 08:31 < andytoshi> i agree we should have put this in the changelog 08:32 < andytoshi> and i also agree that this complicates size estimation for non-segwit txes in the case that you start by estimating a no-input transaction and then add weight per input 08:49 -!- pablomartin [~pablomart@host73.186-108-110.telecom.net.ar] has joined #bitcoin-rust 08:58 < darosior> Yeah i was talking about size estimation, not serialization, my bad for using both words. 11:24 -!- Ademan [~ademan@47.185.95.178] has joined #bitcoin-rust 13:29 -!- DarrylTheFish [~DarrylThe@user/DarrylTheFish] has joined #bitcoin-rust 13:48 -!- pablomartin [~pablomart@host73.186-108-110.telecom.net.ar] has quit [Ping timeout: 276 seconds] 15:06 -!- darosior [~darosior@109.205.214.46] has quit [Ping timeout: 264 seconds] 15:16 -!- darosior [~darosior@109.205.214.46] has joined #bitcoin-rust 16:15 < yancy> Isn't weight estimation done in predict weight https://github.com/rust-bitcoin/rust-bitcoin/blob/4b6b55f0cd757690c0a815bd0b79d6c7d9508be1/bitcoin/src/blockdata/transaction.rs#L1237 instead of Transaction::weight() or am I misunderstanding 16:16 < yancy> darosior 19:43 -!- cmc_ [~methos@gateway/tor-sasl/cmc] has quit [Ping timeout: 240 seconds] 19:46 -!- cmc_ [~methos@gateway/tor-sasl/cmc] has joined #bitcoin-rust 21:46 -!- iamzim [~iamzim@c-174-162-220-48.hsd1.ut.comcast.net] has joined #bitcoin-rust 21:56 -!- iamzim [~iamzim@c-174-162-220-48.hsd1.ut.comcast.net] has left #bitcoin-rust [] 22:51 -!- grndslm [~grndslm@99-144-164-205.lightspeed.jcsnms.sbcglobal.net] has quit [Ping timeout: 246 seconds] 22:52 -!- grndslm [~grndslm@99-144-164-205.lightspeed.jcsnms.sbcglobal.net] has joined #bitcoin-rust --- Log closed Fri Jan 05 00:00:22 2024