AaronvanW has quit [Remote host closed the connection]
brunoerg has quit [Ping timeout: 268 seconds]
b_101 has quit [Ping timeout: 260 seconds]
brunoerg has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] FractalEncrypt opened pull request #26558: doc: add tr() descriptor example to deriveaddresses (master...FractalEncrypt) https://github.com/bitcoin/bitcoin/pull/26558
brunoerg has quit [Ping timeout: 260 seconds]
NorrinRadd has joined #bitcoin-core-dev
mikehu44_ has joined #bitcoin-core-dev
jon_atack has joined #bitcoin-core-dev
sudoforge has joined #bitcoin-core-dev
mikehu44 has quit [Ping timeout: 260 seconds]
mikehu44 has joined #bitcoin-core-dev
brunoerg has joined #bitcoin-core-dev
mikehu44_ has quit [Ping timeout: 260 seconds]
brunoerg has quit [Ping timeout: 252 seconds]
b_101 has joined #bitcoin-core-dev
AaronvanW has joined #bitcoin-core-dev
b_101 has quit [Ping timeout: 260 seconds]
brunoerg has joined #bitcoin-core-dev
brunoerg has quit [Ping timeout: 260 seconds]
Guest24 has joined #bitcoin-core-dev
brunoerg has joined #bitcoin-core-dev
brunoerg has quit [Ping timeout: 252 seconds]
Guest24 has quit [Quit: Client closed]
brunoerg has joined #bitcoin-core-dev
brunoerg has quit [Ping timeout: 252 seconds]
<bitcoin-git>
[bitcoin] furszy opened pull request #26559: wallet: bugfix, insane fee tx creation due duplicated selection of the preset inputs (24.x...2022_v24_wallet_sad) https://github.com/bitcoin/bitcoin/pull/26559
mikehu44 has quit [Ping timeout: 260 seconds]
AaronvanW has quit [Ping timeout: 260 seconds]
<instagibbs>
^ ouch
brunoerg has joined #bitcoin-core-dev
<furszy>
yeah, sad :(
AaronvanW has joined #bitcoin-core-dev
brunoerg has quit [Ping timeout: 260 seconds]
<bitcoin-git>
[bitcoin] furszy opened pull request #26560: wallet: bugfix, invalid CoinsResult cached total amount (master...2022_wallet_bugfix_coinsresult) https://github.com/bitcoin/bitcoin/pull/26560
<instagibbs>
+1 for using GUI / hww which displays/cross checks fee :)
hg has joined #bitcoin-core-dev
<fanquake>
if it's not a problem for master, why is the second PR based on the changes which have been opened against the 24.x branch?
<fanquake>
Things in master should be "fixed" first, and then things backported to 24.x. Rather than building a change for master based on bugfixes that apparently aren't relevant?
<furszy>
because I mainly added test coverage that we don't have in master inside that PR. Then, I'm using the fixed CoinsResult::Erase method to fix another bug inside the coin selection tests.
<fanquake>
The same "bugfix" commit is present in both PRs though
<furszy>
yes, that is because I later use the method that is currently not used in master to solve another bug.
<fanquake>
Ok. Well that's even more confusing. So I'd suggest fixing everything / adding test coverage to master first
<furszy>
re "Things in master should be "fixed" first": the thing is, in master #26559 bug is not a problem because we no longer use the CoinsResult::Erase method since #25685 (which is not part of v24).
<gribble>
https://github.com/bitcoin/bitcoin/issues/26559 | [24.x] wallet: bugfix, insane fee tx creation due duplicated selection of the preset inputs by furszy · Pull Request #26559 · bitcoin/bitcoin · GitHub
<gribble>
https://github.com/bitcoin/bitcoin/issues/25685 | wallet: Faster transaction creation by removing pre-set-inputs fetching responsibility from Coin Selection by furszy · Pull Request #25685 · bitcoin/bitcoin · GitHub
b_101 has joined #bitcoin-core-dev
<fanquake>
RIght, but you're still applying the same "bugfix" labelled change to master
<furszy>
ah ok. That is because we still have the buggy function in master. It's currently unused but it is still there.
<fanquake>
That's fine. Even if in master it's become unused, we can apply the bugfix, and then backport that fix to 24.x, pointing to it being fixed in master, and explaining that it didn't actually effect master, as the code has been unused since 25685. However the backport/fix is relevant to 24.x, where the code is still used.
b_101 has quit [Ping timeout: 260 seconds]
brunoerg has joined #bitcoin-core-dev
<furszy>
ok, np on doing it in that way. The only confusing thing there is the dangerous bug explanation that will only appear on the v24.0 PR that comes after 26560.
<fanquake>
Is that a problem if the dangerous bug only applies to the 24.x branch?
<fanquake>
We can still link to the 24.x issue from the master branch PR, and make reviewers aware of the underlying issue being fixed.
b_101 has joined #bitcoin-core-dev
brunoerg has quit [Ping timeout: 260 seconds]
<furszy>
yeah sure
<furszy>
What I mean, for reviewers that only check 26560 they will think "this guy is fixing a currently dead code to use it inside tests that have issues".
<furszy>
when that commit is actually being used in v24.0 to solve a bad problem.
<furszy>
thus why I feel it weird
<furszy>
but it's all matter of writing a good PR description
<fanquake>
Yes, we can make it clear to reviewers that isn't the case.
<fanquake>
Also confusing is why we have completely dead code in master at all.
pablomartin has joined #bitcoin-core-dev
b_101 has quit [Ping timeout: 260 seconds]
<furszy>
well, in favor of that dead code, have to say that I got enlightened after trying to use it to solve another bug.
<furszy>
but yeah
jespada has quit [Read error: Connection reset by peer]
brunoerg has joined #bitcoin-core-dev
brunoerg has quit [Ping timeout: 256 seconds]
brunoerg has joined #bitcoin-core-dev
brunoerg has quit [Ping timeout: 260 seconds]
brunoerg has joined #bitcoin-core-dev
<achow101>
I'm surprised tests didn't catch this
brunoerg has quit [Ping timeout: 260 seconds]
brunoerg has joined #bitcoin-core-dev
jespada has joined #bitcoin-core-dev
hernanmarino_ has joined #bitcoin-core-dev
<furszy>
all the tests that uses preset inputs only select one coin. So, the Erase function always work fine for them.
<furszy>
the annoying part is that we didn't catch this during the review process but well, shit happens.
b_101 has joined #bitcoin-core-dev
b_101 has quit [Ping timeout: 260 seconds]
szkl has joined #bitcoin-core-dev
NorrinRadd has quit [Ping timeout: 256 seconds]
NorrinRadd has joined #bitcoin-core-dev
ccdle12 has joined #bitcoin-core-dev
b_101 has joined #bitcoin-core-dev
Guest582 has joined #bitcoin-core-dev
yashraj has joined #bitcoin-core-dev
Guest582 has quit [Client Quit]
Guest5852 has joined #bitcoin-core-dev
Guest58100 has joined #bitcoin-core-dev
Guest5852 has quit [Client Quit]
Guest58100 has quit [Client Quit]
jespada has quit [Read error: Connection reset by peer]