--- 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