bitdex has quit [Remote host closed the connection]
bitdex has joined #bitcoin-core-dev
jespada has quit [Ping timeout: 260 seconds]
adil has joined #bitcoin-core-dev
adil has quit [Quit: adil]
Earnestly has quit [Ping timeout: 252 seconds]
dzxzg has quit [Quit: Konversation terminated!]
zeropoint has quit [Quit: leaving]
bitdex has quit [Ping timeout: 244 seconds]
bitdex has joined #bitcoin-core-dev
wafflefrie has quit [Quit: Connection closed for inactivity]
bitdex has quit [Read error: Connection reset by peer]
bitdex has joined #bitcoin-core-dev
Christoph_ has joined #bitcoin-core-dev
Christoph_ has quit [Ping timeout: 248 seconds]
robobub has joined #bitcoin-core-dev
cmirror has quit [Remote host closed the connection]
cmirror has joined #bitcoin-core-dev
wafflefrie has joined #bitcoin-core-dev
Randolf has joined #bitcoin-core-dev
<bitcoin-git>
[gui-qml] johnny9 opened pull request #465: Show "Select Wallet" or "Add Wallet" in the Wallet selector if a wallet isn't loaded (main...no-wallets-loaded) https://github.com/bitcoin-core/gui-qml/pull/465
bitdex has quit [Quit: = ""]
abubakarsadiq has quit [Quit: Connection closed for inactivity]
kevkevin has quit [Remote host closed the connection]
Randolf has quit [Ping timeout: 248 seconds]
Randolf has joined #bitcoin-core-dev
Randolf has quit [Quit: Leaving]
kevkevin has joined #bitcoin-core-dev
kevkevin has quit [Ping timeout: 248 seconds]
kevkevin has joined #bitcoin-core-dev
kevkevin has quit [Ping timeout: 268 seconds]
kevkevin has joined #bitcoin-core-dev
kevkevin has quit [Ping timeout: 252 seconds]
SpellChecker has joined #bitcoin-core-dev
Guyver2 has joined #bitcoin-core-dev
Guest38 has joined #bitcoin-core-dev
abubakarsadiq has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] maflcko opened pull request #32680: ci: Rewrite test-each-commit as rust script (master...2506-ci-rust-script) https://github.com/bitcoin/bitcoin/pull/32680
kevkevin has joined #bitcoin-core-dev
Guest38 has quit [Quit: Client closed]
<bitcoin-git>
[gui-qml] GBKS opened pull request #466: Visual tweaks to the fee selector in the send form (main...feature/fee-selection-visual-tweaks) https://github.com/bitcoin-core/gui-qml/pull/466
kevkevin has quit [Ping timeout: 252 seconds]
wafflefrie has quit [Quit: Connection closed for inactivity]
<bitcoin-git>
bitcoin/master 4ce5349 ismaelsadeeq: doc: update tor docs to use bitcoind binary from path
<bitcoin-git>
bitcoin/master 6a2ff67 merge-script: Merge bitcoin/bitcoin#32679: doc: update tor docs to use bitcoind binary f...
<bitcoin-git>
[bitcoin] fanquake merged pull request #32679: doc: update tor docs to use bitcoind binary from path (master...06-2025-fix-tor-docs) https://github.com/bitcoin/bitcoin/pull/32679
<dergoegge>
I think ideally it'd be 2 PRs: 1. switch to multi index while leaving the interfaces untouched 2. change behavior
<glozow>
dergoegge: yeah sad, but if I reimplement the old eviction strategy as a multi index, it needs like twice as many indexes
<glozow>
you could first review the old version where the new eviction strategy is implemented using the existing data structures if that's more familiar
<sipa>
FWIW, we discussed this and my thinking was that separating the impl/behavior change isn't really worth it for review, if the result is a substantial amount of code that would be added just to be deleted immediately after
<hebasto>
sipa: the "Actions" tab is now available, but it seems not working until a workflow description become part of the default branch :(
Christoph_ has joined #bitcoin-core-dev
<sipa>
hebasto: i'm perhaps not the right person to help you
<dergoegge>
marcofleon was gonna deferentially fuzz the old and new (multi index) version but like this its not possible
<glozow>
porting the existing eviction strategy would need interfaces <ByWtxid> <ByPeer> <ByExpiry> and a random-access for eviction. Then we'd introduce a sequence number, then delete half of the indexes... it just didn't seem worth it.
<glozow>
why not?
<dergoegge>
because the behavior changed
<glozow>
the behavior is supposed to change though?
<marcofleon>
I was gonna differential fuzz the refactor only
<glozow>
wait do you mean the old PR or orphanage on master?
<sipa>
dergoegge: i think what you'd really want for differential fuzzing is first implement the new behavior without multi-index, then add a multi-index version, then fuzz the difference between the two, and then delete/switch over
<dergoegge>
current orphange against multi index refactor
kevkevin has joined #bitcoin-core-dev
<sipa>
because otherwise all you're doing is fuzzing an implementation using two data structures of behavior that's going away anyway
<glozow>
why not fuzz the old version of the PR against the multi index version?
<dergoegge>
i mean that seems different but probably useful too
<marcofleon>
old pr vs new pr could work, that makes sense
<sipa>
i'm not sure that's all that useful, unless it is somehow the case that the old PR is more obviously correctly implements the desired behavior than the new PR
<marcofleon>
basically differential fuzz the new behavior, one without multi-index and one with
<glozow>
If we really want this, I can write it. But it's a bunch more code to review that then goes away immediately. I don't think it's meaningful to differentially fuzz against behavior that we know is subpar and want to change
<glozow>
Like, we are doing this because we know the eviction sucks and want to change it
<sipa>
the point of such a fuzz test is to increase reviewer confidence by establishing that something more-obviously-correct and something less-obviously-correct are equivalent
<sipa>
but i don't think that's the case here, the multi-index makes it a lot easier to reason about
<glozow>
If we want to do 2 stages, we could do the old version of the PR and then the refactor after...?
<instagibbs>
hmm, I'm not sure I found the non-multi index version of the PR significantly easier to read?
<instagibbs>
Unless it is just trivially badly done performance wise then swapped out
<sipa>
instagibbs: that's my point - i think the multi-index one is easier to read, so differential fuzzing with something else isn't really a gain
<dergoegge>
glozow: right that makes sense, I wasn't aware it'd be much more code
<dergoegge>
I was under the impression that the multi index suggestion came up in the context of reviewing the behavior change (i.e. it'd be easier if it was a multi index)
<dergoegge>
Therefore to me it seemed useful to first make sure the switch to multi-index was correct
<instagibbs>
sipa ah ok +1
<dergoegge>
old vs new pr is good, it should be the same thing
<sipa>
but did anyone fully review the old one?
<sipa>
if there is no reason to have confidence that the old one was implementing the desired behavior, i don't think i see the point
<instagibbs>
they're gonna be subtly different too, FIFO eviction vs random from Dosiest peer
<instagibbs>
for one
<sipa>
if anything, you'd want a separate test-only even-more-trivial implementation of the exactly desired behavior, and then have a fuzz test that compares the real implementation with that
<dergoegge>
sipa: right if the fuzzing doesn't turn anything up there is no gain but I think if there is a difference then that at least lets us decide which one is correct/desireable
<instagibbs>
sipa yeah that was my one possible suggestion, it recalculates everythign in some really dumb wy
eugenesiegel has joined #bitcoin-core-dev
<sipa>
(this is what txrequest did/does, it also switched implementation and behavior at the same time, but also added a trivial reimplementation and comparison with it)
<sipa>
but i think the complexity in the implementation in txorphanage is a lot less, so i'm not sure it's worth the effort
<sipa>
still, i think that's more valuable than a comparison with a more complex, more error-prone, never deployed, only partially reviewed, older implementation of the same thing
<instagibbs>
fwiw the introduced fuzz targets didn't have to change much, at least the "one honest package" one didn't
<marcofleon>
txorphan_protected you mean?
<instagibbs>
sounds right
<dergoegge>
instagibbs: ok if we know there are subtle differences already, then there truly is no point
<instagibbs>
whispering more fuzz harness suggestions to marcofleon
jespada has quit [Quit: My Mac has gone to sleep. ZZZzzz…]
<achow101>
A reviewer pointed out that the PR did not implement the restriction specified in BIP 390 that musig() descriptors cannot contain duplicate participants. When I started implementing it, I realized that this restriction is a bit more complicated than I had thought it would be, so I have proposed to the mailing list to drop that.
<achow101>
I forsee some more BIP work will need to be done soon(tm)
<achow101>
#topic orphan resolution WG Update (glozow)
<glozow>
#31829 is up for review, thanks people who have taken a look
<glozow>
I'm open to splitting it, but don't see a good way to do so. lmk if people have ideas
<glozow>
Maybe we could do the prep commits in a separate PR? idk
<glozow>
that's it from me
<achow101>
#topic QML GUI WG Update (jarolrod, johnny9dev)
<johnny9dev>
A lot of QML work getting completed and PRs opened.
<johnny9dev>
Addtional init loading animations were merged in bitcoin-core/gui-qml#460
<johnny9dev>
Standard fee selection control was added to Send to set very basic (low/default/high) block-based fees. This has been merged bitcoin-core/gui-qml#461
<johnny9dev>
Some cleanup to our layouts are being done by goqusan bitcoin-core/gui-qml#463. He's been working through what we have and fixing up messy anchors and layouts and his reviews have helped give clarity on getting the layouts where they need to be
<johnny9dev>
Multiple Recipients in the Send form has been PR'd and close to being merged bitcoin-core/gui-qml#450
<johnny9dev>
First set of form input validation for Send has been PR'd. This will also be used for payment requests. bitcoin-core/gui-qml#462
<johnny9dev>
RequestPayment page has finally been hooked up to our wallet model to generate and commit actual payment requests and addresses. bitcoin-core/gui-qml#464
<corebot>
https://github.com/bitcoin-core/gui-qml/issues/465 | Show "Select Wallet" or "Add Wallet" in the Wallet selector if a wallet isn't loaded by johnny9 · Pull Request #465 · bitcoin-core/gui-qml · GitHub
<johnny9dev>
Momentum is really starting to pick up which is really encouraging
<sipa>
cool
<fanquake>
Have we rebased onto CMake / Qt 6 now?
<johnny9dev>
One issue I'm having and need some help with is related to managing issues on the repo itself. I'm unable to change state on them myself and its slowing us down a bit
<johnny9dev>
I plan on starting build/depends work in a couple of weeks. It will likely start in july
<achow101>
johnny9dev: we can give you triage permissions for that
<johnny9dev>
by build/depends I mean that I will be looking at the best way to update to the latest systems and check on how to properly separate concerns with this project
<jarolrod>
pinheadmz: should have an update on the sync
<sipa>
he's OOO today
jespada has quit [Quit: My Mac has gone to sleep. ZZZzzz…]
<jarolrod>
🌴🌴🌴
<johnny9dev>
triage permissions would help a lot. thank you
<johnny9dev>
thats all i have for this week
<achow101>
#topic Script Validation WG Update (fjahr)
<achow101>
perhaps fjahr is not here
<achow101>
Any other topics to discuss this week?
<fjahr>
sorry, i had connection issues
<fjahr>
Since I last reported here in batch validation (#29491) I have addressed some comments and the latest benchmarks look very promising. Also added some unit tests. I hope I can finally fix the taproot test soon and take it out of draft status afterwards.
<bitcoin-git>
qa-assets/main ef94b3a maflcko: Merge pull request #226 from marcofleon/coins-view-db-inputs
jespada has joined #bitcoin-core-dev
jespada has quit [Client Quit]
<gmaxwell>
fjahr: if you're seeing slowdowns in batch validation with more threads that persist after resolving locking issues there are some internal parameters in libsecp256k1 that influence the cache footprint that might not be optimal because they were tuned by single thread testing.
Earnestly has joined #bitcoin-core-dev
<gmaxwell>
I see in the PR some comments about merging batches-- probably could be done in the API (due to lack of use it hasn't had a lot of development) but I think you don't want to anyways.. the asymptotic batching gain is only like 2x or whatnot, so it'll usually be better for performance to have more concurrency than more batching.
<gmaxwell>
instead you'll probably want to get IBD validation able to batch validate multiple blocks at once eventually... and then even with a zillion cores all the batches will be reasonably large.
dviola has quit [Ping timeout: 248 seconds]
jon_atack has quit [Ping timeout: 252 seconds]
jespada has joined #bitcoin-core-dev
jadi has joined #bitcoin-core-dev
certiorari has quit [Remote host closed the connection]
jonatack has joined #bitcoin-core-dev
spynxic has quit [Read error: Connection reset by peer]
spynxic has joined #bitcoin-core-dev
spynxic has quit [Read error: Connection reset by peer]
spynxic has joined #bitcoin-core-dev
Guyver2 has left #bitcoin-core-dev [Closing Window]
<bitcoin-git>
[bitcoin] PeterWrighten opened pull request #32685: wallet: Allow read-only database access for info and dump commands (master...wallet-readonly-access) https://github.com/bitcoin/bitcoin/pull/32685
jespada has quit [Quit: My Mac has gone to sleep. ZZZzzz…]
jespada has joined #bitcoin-core-dev
SpellChecker_ has joined #bitcoin-core-dev
ghost43_ has joined #bitcoin-core-dev
SpellChecker has quit [Ping timeout: 244 seconds]
ghost43 has quit [Remote host closed the connection]
jonatack has quit [Quit: WeeChat 4.6.3]
jonatack has joined #bitcoin-core-dev
jespada has quit [Ping timeout: 260 seconds]
jespada has joined #bitcoin-core-dev
wafflefrie has joined #bitcoin-core-dev
<fjahr>
gmaxwell: Cool, thanks for the feedback. Currently, the secp PR is still missing usage of Pippenger which is why I am mostly looking at relative improvements in benchmarks by changes in the core PR. After pippenger is added the caching would be good to check to look at next. I suggested the merging because I seemed like a good idea in my head but got the feedback that merging scratch spaces would be needed for that and it
<fjahr>
sounded like something that would take a lot of effort with unclear gains, so might be reconsidered later if it's even worth it at all like you are saying.
<gmaxwell>
yeah my point is that like, if you have 16 cpus you should have 16 batches, because doing say, 8 somewhat larger batches will always be slower than using all the cores. (or even 15 slightly larger batches vs 16 smaller using all the cores). I think the only reason you'd ever want to merge batches is if for some reason you had more validation threads than cores.
<gmaxwell>
as far as merging them, I think it wouldn't actually be complicated, you'd just copy all of one into the other, keeping the destination scratch space size the same.
<gmaxwell>
but I don't think it would actually be useful.