<bitcoin-git>
[bitcoin] theStack opened pull request #23079: test: use MiniWallet for p2p_filter.py (master...202109-test-use_MiniWallet_for_p2p_filter) https://github.com/bitcoin/bitcoin/pull/23079
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] brunoerg opened pull request #23080: test: check abandoned tx in listsinceblock (master...2021-09-improv-test-listsinceblock) https://github.com/bitcoin/bitcoin/pull/23080
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
gene has quit [Quit: gene]
c_arc has joined #bitcoin-core-dev
VH has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
VH has quit [Quit: Client closed]
c_arc has joined #bitcoin-core-dev
bitdex has joined #bitcoin-core-dev
dermoth has quit [Ping timeout: 260 seconds]
c_arc has joined #bitcoin-core-dev
dermoth has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
cmirror has quit [Remote host closed the connection]
cmirror has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc_ has joined #bitcoin-core-dev
cmirror has quit [Ping timeout: 265 seconds]
sdaftuar1 has quit [Ping timeout: 268 seconds]
c_arc has quit [Ping timeout: 252 seconds]
sdaftuar1 has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[gui] jarolrod opened pull request #430: Improvements to the open up transaction in third-party link action (master...3party-tx-links-cleanup) https://github.com/bitcoin-core/gui/pull/430
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
Aaronvan_ has quit [Remote host closed the connection]
c_arc has joined #bitcoin-core-dev
klementtan has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
earnestly has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
tla2k21 has quit [Ping timeout: 276 seconds]
vasild has quit [Ping timeout: 276 seconds]
vasild has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
tla2k21 has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] MarcoFalke opened pull request #23083: rpc: Fail to return undocumented or misdocumented JSON (master...2108-docRpc) https://github.com/bitcoin/bitcoin/pull/23083
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
kexkey has quit [Ping timeout: 246 seconds]
kexkey has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
klementtan has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
c_arc has joined #bitcoin-core-dev
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] MarcoFalke closed pull request #23083: rpc: Fail to return undocumented or misdocumented JSON (master...2108-docRpc) https://github.com/bitcoin/bitcoin/pull/23083
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] MarcoFalke reopened pull request #23083: rpc: Fail to return undocumented or misdocumented JSON (master...2108-docRpc) https://github.com/bitcoin/bitcoin/pull/23083
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
jonatack has quit [Ping timeout: 260 seconds]
c_arc has joined #bitcoin-core-dev
jespada has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
jonatack has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] jonatack opened pull request #23084: test: assert on result instead of log in asmap-addrman test (master...asmap-addrman-test-assert-on-result-instead-of-log) https://github.com/bitcoin/bitcoin/pull/23084
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
<laanwj>
can someone who uses signet please test #23021 and sign off on it; Sjors' old node is no longer active so it's fairly important to replace it
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
c_arc has joined #bitcoin-core-dev
c_arc has joined #bitcoin-core-dev
promag has quit [Remote host closed the connection]
promag has joined #bitcoin-core-dev
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] hebasto opened pull request #23089: ci: Increase the dynamic port range to the maximum on native Windows (master...210924-portrange) https://github.com/bitcoin/bitcoin/pull/23089
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
<meshcollider>
I don't think there are any proposed topics, last minute proposals?
<michaelfolkson>
I'll ask a couple of questions on achow101's recent Twitch streams/PRs that he's been working on if nothing else
<meshcollider>
After another user earlier this week asking about upgrading an old wallet and why they're still only getting legacy addresses, I was wondering if we should add an easier way to fully flush the keypool after an upgrade
<meshcollider>
achow101: what's the reason we keep using the old pre-HD keypool after upgrade until it has run out? Why not just use the new HD keys immediately?
<achow101>
it wouldn't be particularly hard to add a function like "newkeypool"
<meshcollider>
Yeah it'd be super easy, could overload keypoolrefill too, but I wanted to discuss the rationale first
<sipa>
fwiw, when upgrading from unencrypted to encypted wallet, this already happens
<sipa>
so i'm sure that code exists
<achow101>
meshcollider: I don't remember, would have to check the original PR
<achow101>
sipa: there's a convenient NewKeypool() function :)
<meshcollider>
So I feel like we should change it now to flush the keypool on upgrade from non-HD to HD
<meshcollider>
Or even after every upgrade?
<achow101>
oh, I think part of it was that it was basically impossible to show a warning to users that they had to make a new backup after -upgradewallet
<achow101>
but that's no longer relevant with the upgradewallet RPC
<achow101>
I think it would be reasonable to have upgradewallet flush after upgrading to HD
<meshcollider>
Cool, I'll open a PR after the meeting then
<achow101>
as well as a dedicated flush keypool RPC
<meshcollider>
prayank: Sure, what was your question :)
<prayank>
I wanted to start working on things explained in this issue and create a draft PR but don't want to waste time and end up with NACKs in few minutes. So what are your thoughts on it?
<michaelfolkson>
Always upsides and downsides but one downside appears to be reduction in the success rate of BnB
<michaelfolkson>
Less UTXOs at its disposal
<prayank>
Let me explain with one example
<achow101>
I would generally avoid automatically creating new wallet files when certain behavior occurs
<achow101>
a "sub wallet" should just be an internal structure
<prayank>
Alice creates a new address. Bob sends multiple times to it. Becomes dirty. Detected by Core and moved to New wallet with keys and all meta data.
<michaelfolkson>
achow101: Can a subwallet with UTXOs that can't be spent by the original wallet fit into that "internal structure"? I'm guessing not
<michaelfolkson>
The divide is pretty clear
<achow101>
michaelfolkson: it's a program, you can make anything work :)
<michaelfolkson>
Ha ok, I don't know what you mean by an internal structure then ;)
<prayank>
michaelfolkson: Maybe not show user a new wallet is created
<achow101>
with meshcollider's persistent utxo locks, it seems like it would be fairly easy to detect these "dirty utxos" and apply a utxo lock on them
<meshcollider>
Doesn't avoid_reuse basically do that anyway
<achow101>
later users can go in, manually unlock those utxos, and do whatever with them. I think that approximately achieves what #22424 describes
<meshcollider>
Could possibly make it easier to do so with an RPC "consolidatedirty" or something
<michaelfolkson>
meshcollider: Isn't avoid_reuse avoidance rather than making it impossible?
<achow101>
but in general, I don't particularly like the idea of outright disabling users from spending certain UTXOs just because they are reused
<achow101>
it is trivial to lock people out of funds by dust spam
<achow101>
s/is/becomes
<michaelfolkson>
To me avoidance seems fine. Literally dumping the private key so that the original wallet can't spend a UTXO in any circumstances seems overkill
<prayank>
meshcollider: avoid_reuse has issues
<prayank>
achow101: Won't be default. Only for user who want this level of privacy can use it.
<achow101>
prayank: perhaps improve avoid_reuse rather than throwing it out?
<michaelfolkson>
prayank: What are the issues?
<meshcollider>
I like achow101's suggestion of simply locking the UTXOs, but I think then there should be some way to distinguish these types of UTXO for when you do want to consolidate (e.g. red on GUI as suggested)
<prayank>
achow101: well 2 issues that exist are 1. Unable to spend unconfirmed UTXO 2. OUTPUT_GROUP_MAX_ENTRIES = 100 and I was told both will get NACKs if I try to fix them
<achow101>
meshcollider: we could add a "lock reason"
<achow101>
prayank: that's avoidpartialspends, not avoid_reuse
<achow101>
there is a subtle difference IIRC
<prayank>
Okay they looked similar things maybe I misunderstood. avoid_reuse is a just a flag right in wallet?
<meshcollider>
Yes
<meshcollider>
WALLET_FLAG_AVOID_REUSE
<jonatack>
hi
<achow101>
avoid_reuse will cause coin selection to exclude UTXOs that have already been spent previously
<prayank>
jonatack: any opinion on this?
<jonatack>
prayank: maybe have a look at src/wallet-init.cpp:47 regarding avoidpartialspends, and grep for it in the codebase
<achow101>
so it seems like having a wallet with avoid_reuse already solves the problem?
<meshcollider>
I think a consolidatedirty RPC could still be useful
VH has quit [Quit: Client closed]
<meshcollider>
No actually that would basically be impossible to do well
<meshcollider>
Is there an easy way to list the dirty UTXOs being avoided by avoid_reuse?
<achow101>
meshcollider: listunspent has a "reused" value
<meshcollider>
True, okay, in that case it should still be possible to manually run them through a mixer or something if desired
<achow101>
in any case, I think prayank needs to have a look at avoid_reuse in combination with avoidpartialspends first
<achow101>
before we have any further discussions with changes to make
<prayank>
Okay I will check these things in detail
<prayank>
Thanks everyone
<meshcollider>
prayank: thanks! Glad you have some good ideas for this though :)
<meshcollider>
Any other topics?
<michaelfolkson>
A couple of quick ones
<meshcollider>
michaelfolkson: Go ahead
<michaelfolkson>
One, any way to speed up the Proposed Timeline for Legacy Wallet and BDB removal?
<jonatack>
don't worry prayank, it takes time, gathering and slowly accumulating context helps
<achow101>
maybe, but it's intentionally slow and drawn out
<michaelfolkson>
I just wonder if it is too drawn out :)
<achow101>
the point is to ease the transition, especially since the migration might not migrate everything
<prayank>
jonatack: True :)
<achow101>
(it is difficult to replicate everything that ismine would match on)
<michaelfolkson>
achow101: Oh the migration might not... ok I didn't consider that
<michaelfolkson>
Ok that is a reason for being extra cautious
<meshcollider>
michaelfolkson: why would you like it brought forward? Just code legacy code maintaince reasons?
<achow101>
the idea is that there will be time to migrate while all of the legacy wallet stuff is still there so that if there are any issues, a backup can be used and the user could just make a new descriptor wallet and move coins
<sipa>
also, we regularly see people on stackexchange trying to recover funds from wallet.dat files from 2013 or older
<achow101>
also, 2024 is not as far away as you think it is :p
<michaelfolkson>
Ok fair enough, leave as is
<sipa>
it's closer to today than segwit's activation
<meshcollider>
Scary thought
luke-jr has joined #bitcoin-core-dev
<michaelfolkson>
And second question, what was the point of the coin selection simulations? Just see how often the different algorithms were used?
<achow101>
the coin selection simulations are to compare how the algorithms perform
<achow101>
with perform being somewhat subjective hence all of the data output by the simulation
<michaelfolkson>
To check that the preference for BnB makes sense? And check that Knapsack and SRD are equal second preference?
<michaelfolkson>
The conclusion was SRD wasn't quite as good as we thought but not materially enough to change that preference ordering?
<achow101>
not really check anything specific
<meshcollider>
There is no preferred order after the new waste metric approach is there
<achow101>
but moreso see what the effects are in general, e.g. how the utxo set is affected, how fees are affected, etc.
<michaelfolkson>
I guess if SRD became third preference it would never get used. Knapsack never fails right?
<achow101>
with the waste metric, there's no explicit preference
<sipa>
don't forget that these algorithms affect eachother
<sipa>
introducing one means your wallet's UTXO composition changes
<sipa>
which may affect the behavior of the other algorithms
<michaelfolkson>
Hmm ok interesting
<achow101>
the point of the simulations is to compare the different strategies we could use and see how they behave with a common dataset and common metrics
<achow101>
and then somewhat try to rationalize why we see certain behaviors
<michaelfolkson>
Ok cool, makes sense
<michaelfolkson>
Ok I'm done, thanks
<achow101>
I have some old gists with other simulation results from a variety of combinations of BnB + knpsack and other things that were proposed in the past
<achow101>
with some things like different min change targets, different feerates, etc.
<michaelfolkson>
I knew it wasn't pointless :) Just didn't know what the major objective(s) was
<meshcollider>
Just briefly, re: #23065, do people think all unlocks should remove both persistent and non-persistent locks, or should it be possible to temporarily unlock a persistent lock
<michaelfolkson>
Thanks meshcollider achow101 et al
<achow101>
#22009
<gribble>
https://github.com/bitcoin/bitcoin/issues/22009 | wallet: Decide which coin selection solution to use based on waste metric by achow101 · Pull Request #22009 · bitcoin/bitcoin · GitHub
<michaelfolkson>
Ok I don't understand how there can be no preference but I guess I need to understand #22009
<gribble>
https://github.com/bitcoin/bitcoin/issues/22009 | wallet: Decide which coin selection solution to use based on waste metric by achow101 · Pull Request #22009 · bitcoin/bitcoin · GitHub
<meshcollider>
michaelfolkson: it just tries all picks the "best"
<meshcollider>
"best" in this case is defined as the lowest waste
<michaelfolkson>
Ok, BnB was only being used 20 percent of the time from the simulations (presumably that was before the waste metric). Would be much less after the waste metric
<prayank>
meshcollider: Thanks for working on this PR to add persistence. I will have few beers the day it gets merged to celebrate we improved privacy. Although small change but things like this can surely help.
<sipa>
prayank: i really don't understand what reuse persistance has to do with privacy
<sipa>
either you need persistence, or you don't
<sipa>
if you do, you're out of luck without it; if you don't, it has no effect
<prayank>
sipa: lockunspent persistence
<sipa>
sorry, that's what i meant
<sipa>
i see you bringing this up in various places as a major win for privacy, and i don't understand it :)
<sipa>
to be it's just missing functionality being fixed
<sipa>
*to me
<sipa>
s/fixed/added/
<prayank>
Right now it makes locks useless. So privacy is improved with the change.
<sipa>
well, only for people (a) that want to use locks and (b) can't because they're not persistent
<sipa>
is there a particular use case you have in mind?
<prayank>
Yes. Let me explain.
<shiza>
Is that to prevent forced reuse attack, I'm starting to understand your motivation.
<shiza>
If* is that to...
<prayank>
A user can lock 1 UTXO associated with some transaction to avoid using it. After few days user relaunches Bitcoin Core, forgets about lock being removed, sends some bitcoin to Binance in which this UTXO is used. Binance asks for source of funds etc.
<sipa>
ok, but why would they lock anything?
<prayank>
To keep things separate
<sipa>
use separate wallets if you want to keep things separate
<prayank>
I don't want to use same coins for all type of transactions
<sipa>
perhaps, but i expect you're not a representative user
<promag>
^ thats what I suggested in the past
<prayank>
That's a workaround
<sipa>
it's not like suddenly everyone is going to start micromanaging their utxo pool like that
<promag>
separate it's better no? it won't spend from multiple wallets and you don't have to remember to lock
<prayank>
They already do in other wallets.
<michaelfolkson>
Do you lock UTXOs prayank?
<prayank>
Yes
<prayank>
Even new wallets like sparrow has better options
<michaelfolkson>
So it is solving a use case for you as a user
<prayank>
It's fixing a problem which exists since 2012 because maybe all users who care about privacy either started using other wallets or figured out workarounds
<promag>
I had a case where unspents were locked for a short period before actually sending the spending tx, and we wanted it to be resilient to daemon restart
<promag>
for that reason we had to store locks externally
<sipa>
prayank: to me the solution was having multiwallet support
<prayank>
Anyway I am happy and this change motivated me.
<promag>
sipa: one case I think makes sense is when you re-receive on an address and don't want to use that utxo
<achow101>
promag: but avoid_reuse deals with that problem
<promag>
havent seen that one in detail - does it avoid entirely?
<michaelfolkson>
prayank: Yeah seems good. I'm not always convinced by some of your plugging privacy leak suggestions but this seems convincing :)
<achow101>
promag: yes
<achow101>
sipa: perhaps some people want to have UTXO separation but also one place to see their full balance?
<achow101>
there's also possibly something like grouping UTXOs, but they can remove between groups
<achow101>
s/remove/move
<promag>
the gui/cli can sum balances from loaded wallets
<achow101>
promag: where in the gui?
<promag>
somewhere ^^
<jonatack>
for -getinfo someone could pick up #19092