< achow101>
ah, so laanwj forgot to push the branch itself
< jarolrod>
has boost been taking a long time to download using depends recently or is it just me
< martinus>
ankerl
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #22146: Reject invalid coin height and output index when loading assumeutxo (master...2106-auHeight) https://github.com/bitcoin/bitcoin/pull/22146
< bitcoin-git>
bitcoin/master 3d552b0 Sjors Provoost: [doc] explain why CheckBlock() is called before AcceptBlock()
< bitcoin-git>
bitcoin/master a748782 MarcoFalke: Merge bitcoin/bitcoin#15545: [doc] explain why CheckBlock() is called befo...
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #15545: [doc] explain why CheckBlock() is called before AcceptBlock (master...2019/03/clarify-checkblock) https://github.com/bitcoin/bitcoin/pull/15545
< vasild>
michaelfolkson: for me the meetings' time of the day is a showstopper
< michaelfolkson>
vasild: Without doxing your specific location are you Asia Pacific timezone?
< vasild>
michaelfolkson: I am in Central Europe (GMT+2)
< michaelfolkson>
Oh ok, just other commitments in evenings? I am GMT+1 and timings are fine for me
< vasild>
family time, dinner, sleep early :)
< michaelfolkson>
Fair enough :)
< michaelfolkson>
I feel bad for Asia Pacific people, often in the middle of the night for them
< vasild>
yeah, that's even worse
< vasild>
jnewbery: wrt running the fuzzer in CI, I did not follow the discussion closely, was something decided about it? It occurred to me that it makes sense to run the fuzzer on CI on every pull request using the pre-generated seeds if that increases the code coverage. E.g. if without the fuzzer a CI run gets 80% coverage of a pull and if with the fuzzer that gets to e.g. 85% then it makes sense to run
< MarcoFalke>
At least that fixed it for me: " * [new tag] v0.20.2rc2 -> v0.20.2rc2"
< MarcoFalke>
vasild: I'd be a hard NACK removing the fuzz tests. Might as well remove all other tests
< vasild>
I agree, but this was discussed in a meeting recently, IIRC due to some sporadic timeouts
< _aj_>
MarcoFalke: once #21496 is done, could run against the existing fuzz corpus without actually compiling for fuzzing -- could see that being worth a try
< michaelfolkson>
vasild: I quite like the idea of using StackExchange as a staging ground or place for drafts before they are ready to add to Core docs :)
< jonatack>
michaelfolkson: there is a wide range of area within the CET timezone (maybe other timezones too but CET I know well), e.g. sun setting and rising much "earlier" in the eastern parts
< vasild>
michaelfolkson: thanks for the link, I should do that before 22.0 release
< jnewbery>
MarcoFalke: saying "we might as well remove all other tests" is not helpful. The point is that we've loaded so many additional tasks onto the CI that it gives frequent false failures.
< jnewbery>
If developers first thought when seeing a failing CI is "it's probably a spurious failure, I'll hit re-run", then it's worse than useless.
< jnewbery>
and the approach of "let's just spend money and throw more CPU at the problem" isn't a good solution
< hebasto>
it appears that `lupdate` can produce XLIFF file directly; the drawback is that `-locations relative` does not work in this case
< hebasto>
laanwj: luke-jr: ^
< bitcoin-git>
[bitcoin] sdaftuar opened pull request #22147: p2p: Protect last outbound HB compact block peer (master...2021-06-reserve-outbound-hb) https://github.com/bitcoin/bitcoin/pull/22147
< MarcoFalke>
jnewbery: Sure but then the first attempt should be to fix the spurious failure, not to remove the whole test config
< MarcoFalke>
Without any way to run the fuzz tests in CI, they will quickly become stale
< MarcoFalke>
We've already moved the fuzz memory sanitizers out of the ci config. The memory issues are rare enough that they can be fixed up after a merge
< MarcoFalke>
Though the other issues (logic bugs in the fuzz code, other sanitizer issues, ...) are far more frequent and need to be dealt with in the pull that introduces the issue
< MarcoFalke>
If there is a specific fuzz test that is slow and not too helpful, we could remove that one
< MarcoFalke>
Also, we have plenty of intermittent failures in the functional test suite, which are waiting to be fixed
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #22150: test: Remove unused node from feature_nulldummy (master...2106-testFaster) https://github.com/bitcoin/bitcoin/pull/22150
< bitcoin-git>
[bitcoin] hebasto opened pull request #22151: build: Follow Transifex docs to prepare XLIFF source (master...210604-xliff) https://github.com/bitcoin/bitcoin/pull/22151
< ryanofsky>
A compromise short of disabling unreliable tests, could be adding a way to mark them flaky. Flaky tests would still run but just collect debug info and not cause the CI to fail
< ryanofsky>
This would also let us track which tests are unreliable over time
< luke-jr>
hebasto: can we postprocess to remove the locations entirely? (do they serve a purpose?)
< hebasto>
luke-jr: do you mean for the source file?
< hebasto>
probably locations are used by Qt Linguist
< hebasto>
but not sure
< achow101>
#proposedwalletmeetingtopic subtract fee from recipients intended behavior and edge cases
< achow101>
ryanofsky noticed that the current implementation (after 17331 removed the loop) will increase the recipient amounts if a dropped change output was more than the required fee
< achow101>
I looked at the previous looping implementation, and it looks like that would actually just overpay the fees in that case. it would reduce the fee from the recipients, then drop the change output to fee
< achow101>
I think neither of these things are what we want to do
< michaelfolkson>
Your view is... achow101? :)
< sipa>
what does dropped mean here? don't you "start" from a transaction without change, and add it if necessary?
< achow101>
sipa: we start from a transaction with change, then subtract the fees from it. then if it is below dust or the exact match window, we remove the change output
< achow101>
with subtract fee from output, we skip the "subtract fees from it" step
< sipa>
achow101: only in the current knapsack?
< achow101>
sipa: always
< achow101>
we do this regardless of the algorithm
< achow101>
with BnB, the change always ends up in the exact match window since we do the same math, so the change is always dropped
< ryanofsky>
IMO the choice is just between slightly overpaying fees and being pedantic about the word "subtract". Or just keeping current behavior and mostly subtracting sometimes adding to recipient when it's most efficient
< ryanofsky>
This is good for the use-case of transferring funds between wallets, which is at least is where I've used subtract from recipient before
< michaelfolkson>
I do think paying a little more than the recipient is expecting is strange behavior for both the sender and the receiver
< ryanofsky>
It deserves to be documented, sure
< achow101>
what we've always done is overpay fees, probably too much
< sipa>
ryanofsky: i think that's the most important use case
< sipa>
moving all funds from one wallet to another
< ryanofsky>
But I think you choose behavior based on use cases, not on being pedantic about terminology
< achow101>
ryanofsky: the use case I've seen the most is moving all funds from one wallet to another, in which case this discussion doesn't matter
< achow101>
but we don't really know what people actually use this for
< ryanofsky>
achow101, it matters because in one case you overpay the miner, in the other case you have a little more funds in your new wallet
< sipa>
is this a long term problem? i'd imagine that in a BnB + SRD world, you have two algorithms one that aims for no-change solutions, and one that aims for with-change solutions, and you pick the best one
< ryanofsky>
but it doesn't matter that much, it is always a case where amount is less than the estimated cost of creating & spending the utxo
< Guest37>
Im looking for a developer, anyone freelancing?
< sipa>
and if the with-change solution results in too low change, you just discard that solution
< michaelfolkson>
Guest37: Sorry, in the middle of a meeting
< sipa>
or you convert it to a no-change one, and consider it in that form
< jonatack>
use case: i subtractfeefromamount in the case of say, selling btc to a party who chooses the fee rate they want to use for the txn
< Guest37>
its okay, after call me.paying big bucks for a bitcoin competitor. "Bitcoin Green" - Runs 100% sustainable. No electricity used ever.
< sipa>
Guest37: go away
< Guest37>
PM me before u miss the launch
< ryanofsky>
sipa, this is all after coin selection, when the algorithm has already run and then we discover the change output is uneconomical
< Guest37>
*scoff* nerds
< achow101>
I think this case is an extreme edge case
< ryanofsky>
so we discard the change output, and pay slightly more fees in the normal case
< achow101>
especially after effective value
< ryanofsky>
and in the subtract from recipients case we can do a little better and send the extra amount to recipients, which is what the code is doing now
< ryanofsky>
but agree it is not very important
< sipa>
ryanofsky: which means it increases the waste metric (once that's in use), and will hopefully not be favored if an actual no-change solutions exists too
< sipa>
?
< ryanofsky>
Not sure, I'm not familiar with the waste metric yet
< achow101>
sipa: I don't think the waste metric accounts for change being dust, yet
< sipa>
it should, i think
< sipa>
overpaying fee is waste
< achow101>
right, we do consider that when we know there is no change
< achow101>
actually, thinking on it, I don't think knapsack can even find a solution where change would be dust.
< achow101>
since it targets a minimum change value
< ryanofsky>
Then maybe it is even more rare and only happens with manual coin selection
< sipa>
i'm thinking something similar
< achow101>
indeed
< jonatack>
test coverage on this may be nice, if missing
< sipa>
in whatever situation where you'd end up with change that's uneconomical (which dust definitely is), there *should* exist a better no-change solution that BnB would find
< sipa>
at the very least, the one that's exactly the same but with the change converged to fee
< sipa>
(but possibly, a better one too)
< ryanofsky>
Either way, I'm just defending current behavior, and don't see a benefit in changing besides being more strict about "subtract". But I don't think it would be a huge deal to change
< achow101>
ryanofsky: the current behavior is accidental though, and if you look at before 17331, we would always overpay
< ryanofsky>
Ok, I did write the code intentionally that way when I suggested in in the other PR, but maybe I misread previous behavior
< achow101>
I didn't realize that at that time. if I did, we would have had this discussion several weeks ago
< ryanofsky>
I was trying not to change behavior, but I think I was basing it on your existing changes in that PR not the previous code.
< ryanofsky>
Anyway, that would be another reason to change, which is fine
< achow101>
I'll do some further analysis on the actual conditions this could occur and we can revisit this later
< achow101>
as usual, coin selection remains inscrutable
< achow101>
#topic what to prioritize for taproot wallet support? (sipa)
< core-meetingbot>
topic: what to prioritize for taproot wallet support? (sipa)
< ryanofsky>
Sure, and you should really do what you want there ultimately
< michaelfolkson>
Can you give an update on where we are with Taproot wallet support to start with sipa? I saw this PR though haven't looked through it yet https://github.com/bitcoin/bitcoin/pull/21365
< sipa>
so a rough list of missing things in master: signing support, descriptor inference support, multisig support (multi(k,...) doesn't exist in taproot), actual PSBT support with extensions so spending paths can be conveyed, support for easily constructing no-keypath descriptors, ...
< sipa>
do we want to try to get signing support in 22.0?
< achow101>
I think we should try to get signing support for 22.0
< achow101>
and basic descriptor inference
< fjahr>
hell yeah :)
< sipa>
signing for basic stuff is done (which is really only useful for keypath signing, but in theory does script paths too)
< achow101>
michaelfolkson: we have tr() descriptor construction with keys only now. you can import them into a watch only descriptor wallet
< sipa>
so we punt multisig support and other descriptor extensions
< sipa>
and just try to get basic key path signing to work
< achow101>
I think that's reasonable
< achow101>
also 22.0 should be released before taproot activates, so I don't think we need to have support for everything at the very beginning
< sipa>
we'll want PSBT extensions for taproot (and musig, once that's a bit further along), but that's a larger discussion than just the bitcoin core wallet
< michaelfolkson>
So that would just be #21365 that needs review and merging for 22.0?
< michaelfolkson>
When people update it it makes it easier to follow what people are working on wallet related
< achow101>
we could add one
< michaelfolkson>
Sounds like Taproot support is the current priority generally for the wallet
< warren>
BTW did anyone announce the move of #bitcoin-core-dev to Libera? Libera had retweeted those announcements for other projects.
< michaelfolkson>
warren: It was announced in some places, there are always going to be more places it could be announced in. You want a tweet specifically from say a Bitcoin Core Twitter account?
< sipa>
laanwj: want to tweet about the IRC move from bitcoincoreorg?
< laanwj>
sure
< laanwj>
no problem with that, though i generally don't tweet IRC related things, definitely not on the bitcoincoreorg account, it seems more developer-internal than announcement
< ryanofsky>
Wow, I thought the whole reason we used IRC was so no one would find us. Maybe this tweeting thing will turn over a new leaf :)
< sipa>
laanwj: that's fair
< laanwj>
ryanofsky: that
< sipa>
we don't really have "user facing" stuff on IRC, maybe other projects do
< warren>
that's a good point, I guess it matters for the other channels not this
< achow101>
I do remember seeing (and tweeting) a few wtf tweets when freenode closed the channel
< warren>
but there is no "official" account to tweet for the user channels
< laanwj>
if you want to make a "community channel" that's okay but it probably shouldn't be here
< warren>
I retract that question.
< laanwj>
at least with libera we have free well-working bridging to matrix
< laanwj>
so if you want a lot of people here, just ask
< jonatack>
no tweet sgtm
< warren>
Is Matrix good? I hadn't heard about code quality and association with altcoin devs made me suspicious early on.
< laanwj>
yes, matrix is basically discord but free software
< achow101>
sipa: what about bech32m in the gui?
< sipa>
achow101: we probably want that too, but i don't think it's a short term requirement
< sipa>
as long as we don't intend to expose creation of taproot wallets by default
< laanwj>
+e2e encryption
< laanwj>
but not if you IRC bridge ofc
< achow101>
sipa: I suppose right now if you can make a taproot wallet, you can also use the cli to get addresses
< sipa>
right
< sipa>
you'll need RPC to import the descriptor anyway
< laanwj>
warren: not sure where you get the association with altcoin devs from--but in any case it's used in large scale outside cryptocurrency, for example FOSDEM 2021 was done entirely on matrix
< achow101>
sipa: are you going to do both OutputType::BECH32M and infer support, or would you like me to take a look at one (or both) of them?
< sipa>
i'll do inference
< achow101>
ok, I'll do outputtype then
< warren>
laanwj: apparently major early funding came from altcoin ecosystems. glad to hear it doesn't matter now.
< laanwj>
warren: fair enough, it's the same thing that makes me skeptical about radicle as a decentralized github at the moment
< warren>
same problem with signal I guess
< michaelfolkson>
I drafted an example for you sipa, hope you don't mind :)
< laanwj>
i have been trying to keep track of it but really it's too much about tokens and "governance" instead of delivering useful functionality
< michaelfolkson>
Please correct if inaccurate
< laanwj>
yes
< sipa>
who would have thought that money corrupts
< sipa>
"Many solutions were suggested for this problem, but most of these were largely concerned with the movement of small green pieces of paper, which was odd because on the whole it wasn't the small green pieces of paper that were unhappy."
< laanwj>
hehe, yes it's supposed to be a motivation for people to do useful things, i'm not convinced
< michaelfolkson>
Some people manage to stay just as productive post money. Admittedly some don't :)
< hebasto>
observing vandal-like actions in Ukrainian translation on Transifex; made local backups just in case
< sipa>
hebasto: i hope it has history, and changes can be reverted?
< hebasto>
sipa: right, on per-message basis
< hebasto>
I've send a message to that user asking for more attention (in case his three actions in a row were made by accident)
< hebasto>
is 0.20.2rc2 supposed to be signed for windows only, or macos as well?
< achow101>
hebasto: it should have a macos sig too, just waiting on jonasschnelli
< hebasto>
achow101: ok
< achow101>
sipa: what about importing tr() descriptors and bech32m addresses into legacy wallets? I guess it would make sense to be able to watch them with a legacy wallet?
< sipa>
achow101: just watching would be easy i guess, just convert them to watched scripts
< achow101>
but that would also end up watching the p2sh wrapped versions too (I think)
< sipa>
right
< sipa>
that could be avoided with more special casing, i guess
< sipa>
but also doing more (like enabling updating PSBT with that, once there are extensions) will be hard
< achow101>
my thought was to just completely disallow bech32m for legacy wallets
< sipa>
i think that makes sense
< achow101>
which should take care of any future things too
< sipa>
right
< bitcoin-git>
[bitcoin] achow101 opened pull request #22154: Add OutputType::BECH32M and related wallet support for fetching bech32m addresses (master...outputtype-bech32m) https://github.com/bitcoin/bitcoin/pull/22154
< bitcoin-git>
[bitcoin] ryanofsky opened pull request #22155: wallet test: Add test for subtract fee from recipient behavior (master...pr/subfee) https://github.com/bitcoin/bitcoin/pull/22155
< sipa>
achow101: this is a much harder problem than i thought
< sipa>
inferring tr descriptors
< sipa>
fun edge case: what if you have repeated subtrees
< achow101>
sipa: for 22.0, I think we can have just the minimum of "OP_1 <pubkey>" -> "tr(<pubkey>)"
< sipa>
sure
< sipa>
well, that's not correct
< sipa>
even single-key cases get tweaked
< sipa>
but still, that wouldn't be hard to do
< achow101>
but the tweak is reversible?
< sipa>
no
< sipa>
it's similar to P2PKH in a way
< sipa>
you can't infer the pubkey from the scriptPubKey
< sipa>
but if you started from an expanded descriptor, you have enough information in the SigningProvider to infer
< achow101>
ah, without the internal key, you can't get the tweak
< sipa>
right, the tweak is literally just H(internal key)
< achow101>
but with access to the SigningProvider, it is possible to get at least the internal key
< sipa>
the rest too
< sipa>
it's just a bit tricky to infer a tree structure from a bunch of leaves
< achow101>
how is the tree stored in the SigningProvider?
< sipa>
as a map (script -> controlblock)
< sipa>
which is kind of how i expect it to end up in PSBT as well
< sipa>
though obviously the internal structure can change if some other approach is taken
< sipa>
if the internal structure is literally the tree it'd be easier, but i think that would be hard to map to something PSBT-like I think
< sipa>
unless it's a field of the form "here is the serialized tree, go nuts"
< achow101>
that would be the easiest
< achow101>
but perhaps not privacy preserving
< sipa>
right, it'd be nice if it was possible to give "this is the only branch you need to care about"
< achow101>
so the problem is if you have the same script in different parts of the tree
< sipa>
oh
< sipa>
lol
< sipa>
this problem just cannot occur
< achow101>
phew
< sipa>
because to have repeated subtrees, you need repeated leaves
< sipa>
and repeated leaves can't be encoded in a (script -> control) map
< sipa>
it'd need to be a multimap
< achow101>
I was going to point that out
< sipa>
thanks!
< achow101>
why couldn't we have repeated leaves?
< sipa>
in descriptor form they're possible
< sipa>
but not in the SigningProvider form i have now
< sipa>
(they're also pointless...)
< achow101>
so we could just disallow them
< sipa>
yeah, though they can't be detected in descriptor form
< sipa>
only after derivation
< sipa>
e.g. you could have tr(KEY,{xpub/*,pubkey}), and pubkey matches the xpub/* derivation at index 1923098173981