< bitcoin-git>
[bitcoin] sipa opened pull request #22275: A few follow-ups for taproot signing (master...202106_taproot_sign_followup) https://github.com/bitcoin/bitcoin/pull/22275
< fanquake>
yea unfortunately it is non-trivial to get into, or out of, Australia the moment
< amiti>
vasild: thanks for your replies, some follow ups-
< amiti>
“I wasn't aware of your work before yesterday. I think late concerns are normal annoyance in software development, not specific to Bitcoin Core or decentralized projects (I worked 10 years in Oracle, I know!). It is annoying, but being late does not make the concerns automatically less valid.“ - I’m not suggesting that being late means the concerns are invalid. I am asking how we can pause to reflect on the disconnect.
< amiti>
Rather than resign ourselves to the idea that this is just the way things are, I’m asking if we can communicate about how we can improve the way we collaborate as a team.
< amiti>
For example, some things I wonder are - what are your main information streams for learning about ongoing work? (Also applies to the others who said they only recently learned about the work.) Do you usually read meeting logs and missing these ones were an outlier, or are meetings not something you usually pay attention to? My primary methods of communication were via the IRC meetings and the mailing list, so it’s useful
< amiti>
information to know if these aren’t reaching the intended audience. I opened the PR and kept it in a draft state as I researched other clients, do you think this impacted your likelihood of noticing the work? Anyway, the point is, there was clearly a disconnect and I’m interested in understanding how we can learn from it.
< amiti>
vasild: “In the same way you seem to be unaware of previous attempts to fix the black holdes problem and explicit signaling for address
< amiti>
relay.. [examples]” - hm actually, I have seen these conversations, many of them were shared with me during the irc meetings. I reread them and don’t see anything that conflicts with what I’m proposing, so I’m not sure why you are making this claim. Please let me know if there’s something specific you think I am missing.
< amiti>
In regards to the technical concerns of #21528, I’ve responded on the PR in an attempt to keep the conversation a bit cohesive.
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #22277: test: Properly set BIP34 height in CreateNewBlock_validity unit test (master...2106-test34) https://github.com/bitcoin/bitcoin/pull/22277
< vasild>
amiti: Hello! One sure way to bring my attention is to mention my nick on IRC or on github. But I realize that you could not have pinged personally everybody that could be interested. I should read meeting logs, but I dont always do, which is my bad.
< vasild>
Following everything that is going on would be a near-full time experience, just to keep up to date. I don't do it.
< michaelfolkson>
amiti: My two cents. I don't think you could have done any more to ensure everyone was aware of the PR and I can see why you're frustrated. A Bitcoin Core PR review club, bringing it up in P2P meeting(s), bringing it up in Core dev meeting, mailing list post etc
< michaelfolkson>
Hence I'm surprised anyone who follows Core P2P development wasn't aware of the PR. However, I think this is just an unfortunate circumstance where one specific detail of the PR didn't become obvious until very late
< michaelfolkson>
I don't think you could have done any more or that the process needs improving. Just hopefully won't happen again with any kind of regularity
< michaelfolkson>
The vasild suggestion to refer to or post on old related PRs is a good one but a minor improvement on what you've already done
< michaelfolkson>
(imo)
< vasild>
+1
< bitcoin-git>
[bitcoin] glozow closed pull request #22253: validation: distinguish between same tx and same-nonwitness-data tx in mempool (master...2021-06-same-txid-diff-wtxid) https://github.com/bitcoin/bitcoin/pull/22253
< bitcoin-git>
[bitcoin] glozow reopened pull request #22253: validation: distinguish between same tx and same-nonwitness-data tx in mempool (master...2021-06-same-txid-diff-wtxid) https://github.com/bitcoin/bitcoin/pull/22253
< Pree>
Hello everyone myself Preeti Sharma. I'm new to this organization but have a decent knowledge in blockchain domain. While exploring the repo I was quite overwhelmed with the work and want to contribute but need someone to colab with on issues. If anyone up for collaboration or can guide me feel free to connect
< laanwj>
re: #22250 i've had quite a lot of stability issues with i2pd (~monthly segfaults), running git master version now (to get tracebacks) and it hasn't crashed yet, but would be careful to recommend using it
< bitcoin-git>
bitcoin/master 451b96f S3RK: test: kill process group to avoid dangling processes
< bitcoin-git>
bitcoin/master 0844084 MarcoFalke: Merge bitcoin/bitcoin#22249: test: kill process group to avoid dangling pr...
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #22249: test: kill process group to avoid dangling processes when using `--failfast` (master...test_kill_process_group) https://github.com/bitcoin/bitcoin/pull/22249
< vasild>
laanwj: "~monthly segfaults" -- I hope you mean i2pd, not bitcoin core
< vasild>
if you are on freebsd, setting MALLOC_CONF=junk:true,abort:true in the environment helps to get crashes more often :)
< vasild>
Because on Linux and on FreeBSD without this setting, reading a memory soon after it is free()'d seems to work and get the data which was there before the free(), as if free() did not happen.
< vasild>
malloc.conf(3) man has some explanation what those settings do
< laanwj>
in any case, as long as this issue isn't resolved, we probably shouldn't be recommending it
< laanwj>
(so I think your document is okay as-is in that regard)
< vasild>
I deliberately refrained from recommending any particular i2p router. I have no reason to assume that the java one is better.
< laanwj>
java at least doesn't have use after free problems
< vasild>
yes :)
< laanwj>
but this was in the context of jonatack mentioning a minimal version to use
< laanwj>
which would make sense in itself
< vasild>
and also I did not want to turn the bitcoin doc i2p.md into "how to install and configure i2p router", which obviously belongs to that i2p router docs
< laanwj>
sure
< laanwj>
though I also think being helpful in things like that can help to get more I2P nodes running
< laanwj>
unlike tor, i2p is still quite unknown and the existing documentation isn't that great in my experience... not that it means we need to maintain it of course
< jonatack>
yes, i was thinking of adding a section that describes some issues to be aware of...in the case of i2pd, there are three, maybe four different ones AFAICT
< laanwj>
use-after-free can be a serious expoitable issue
< jonatack>
(a) minimum version on debian reported by sdaftuar v2.33 no go, 2.35 ok (b) use after free bug report by vasild, (c) crashes on freebsd seen by laanwj, (d) 2.36 stops working for me after 2-3 bitcoind restarts on debian
< laanwj>
do I get it right (regarding #20234) that it's not possible to not bind a P2P port at all?
< laanwj>
I'm a bit confused about the assert(connOptions.bind_on_any)
< laanwj>
if it gets there and there are no binds at all it will crash with an assertion error, so that's supposed to be impossible
< provoostenator>
Re Core Dev meetup in autumn: good idea. Prague would be nice. Not in North Korea please.
< jonatack>
iirc connOptions.bind_on_any == true means there are no binds
< laanwj>
my interpretation is that it means "bind on all interfaces"
< jonatack>
seems it could use a comment
< laanwj>
if it means no binds, it's not a good name imo
< laanwj>
please make it something like bind_on_none
< vasild>
ah, the Core Dev meeting, if it is in Europe I will most likely attend
< vasild>
laanwj: I have completely forgotten what that does, neet to refresh my memory... :)
< laanwj>
bitcoind -nobind -noconnect still works so it must be that I'm misunderstanding the code
< laanwj>
it does seem bind_on_any is true in that case
< laanwj>
clearly it is not binding on "any" though :)
< laanwj>
oh I mixed it up with -nolisten maybe?
< laanwj>
right: -nobind isn't actually a thing, -nolisten is ... the behavior for -nolisten/listen=0 is different in that things still get added to binds they just don't get actually bound
< laanwj>
so it works out
< laanwj>
(albeit the execution flow might be confusing to some people in that case)
< vasild>
actually what does -nofoo mean if the foo argument is not a boolean but is supposed to take string arguments? e.g. -foo="askjdh"
< laanwj>
it used to be that -nofoo is always equivalent to -foo=0 , so -nobind would be -bind=0 which is senseless, but i don't think that is the case anymore? i didn't see any bind arguments passed in
< lightlike>
there is this user/bot "mostafarahimifard" who goes over PRs and likes every post in them (e.g. in 22261, 22253, 15228 etc.). i guess it's harmless, but distracting.
< lightlike>
havent seen any actual contribution by them
< laanwj>
lightlike: thanks, let's see
< jonatack>
lightlike: laanwj: yes, see my review comments in 20234 for example
< laanwj>
we have a fan !
< jonatack>
it's cute :)
< jamesob>
Reading over old docs... remember when we had things called pcoinsTip and chainActive lol
< lightlike>
to add to the confusion, the combination "-nobind=0" will get converted to "-bind=1" by the ParameterParser. E.g. "-noconnect=0" results in the node trying to connect to a peer with IP "1".
< sipa>
haha
< laanwj>
i've argued in the past that -noXXX shouldn't be allowed to have arguments because it's just confusing
< sipa>
yeah
< laanwj>
i think it was kept out of some backward compatibility concern but i dunno, seems more confusing than is worth it
< jonatack>
agree. i don't use them. at least on startup the debug log begins by printing the passed config options (thanks larryruane!) and i sometimes look at what it parsed
< laanwj>
in any case it think the distinction between "what to bind" and "whether to bind at all" makes sense in this case, and is definitely not something that should be changed in #20234, i was just confused for a minute how it could possibly work
< jonatack>
and noconnect=0 -> Command-line arg: connect=true ... Binding RPC on address ::1 port 38332 failed. (no connections at all)
< vasild>
we should add support for multiple negations, like -nonofoo should mean -foo
< laanwj>
-nonononononononono
< vasild>
:-D
< sipa>
i think we should add a -nonogram=file option that dumps a pixelated image representing your config
< laanwj>
i think another reason that we've been very loose with that is that option parsing used to be very loose, e.g. as in there didn't even use to be a path for reporting parse errors from downstream, with the new framework it should be possible
< sipa>
which can of course be disabled using -nogram
< laanwj>
lol!
< vasild>
laanwj: "it's not possible to not bind a P2P port at all" -- https://github.com/bitcoin/bitcoin/pull/20234 is not supposed to change this behavior which is - no, you can't specify "don't bind at all" using -bind=... if -bind is not given then we bind on 0.0.0.0, if -bind=foo is given then we bind on foo
< laanwj>
vasild: I realize this now
< vasild>
however -listen=0 would achieve that
< vasild>
I just tried what would happen if I provide invalid arg to bind like -bind=1.2.3.4:1234 and it quit with this useful message: Error: Unable to bind to 1.2.3.4:1234 on this computer (bind returned error Can't assign requested address (49)) ... Failed to listen on any port. Use -listen=0 if you want this.
< laanwj>
I was right in the sense it's not possible to do that with -bind, but yes, there's -listen for that, so it's fine
< laanwj>
vasild: do you mean 'there should always be at least one outgoing I2P connection if the user has set up I2P' (similar for Tor)?
< vasild>
no actually, it is a broader question: should we try to establish at least one I2P (or Tor) connection if the user has bothered to setup the relevant proxy which includes two things: put the seeds in addrman (I am not sure if they are not put by default?) and then take such an address from addrman and try to connect to it
< vasild>
yes, this is what I mean
< vasild>
or well, if there are already incoming ones, then maybe not bother
< laanwj>
I'm not actually sure about that, maybe from a 'don't become isolated' point of view it makes sense
< vasild>
laanwj: s/outgoing/ in your comment
< vasild>
s/outgoing//
< laanwj>
if you set up all overlay networks then making sure there is a conenction to all of them makes eclipse harder
< sipa>
i think a "protect at least one peer of each network" is probably less invasive
< laanwj>
but there might be drawbacks too
< laanwj>
yes
< vasild>
sipa: we already protect existent connections (per network), the thing is, if we dont have any (to a given network).
< sipa>
yes, i understand the difference
< laanwj>
vasild: i guess that applies only to the case where the user adds an overlay network later, while not having addresses for them yet
< laanwj>
which should be relatively rare (except now in the beginning)
< vasild>
I am not sure either, but it just does not seem right if a user has bothered to setup I2P proxy and configure bitcoin core to use it and to have 0 i2p connections (same for tor)
< laanwj>
yes, though adding such functionality for 0.22 is going to be too late anyway, is this still a concern by 0.23?
< vasild>
maybe not, if it always happens to have some tor connections and having 0 connections is only issue for i2p, then we can assume that in the future it will get better and we can do nothing
< vasild>
was just a random thought
< laanwj>
but regarding seeding per network, might make sense, i dunno
< * vasild>
afk
< laanwj>
to be clar, 'we have zero I2P peers and want to connect to I2P so connect to the seed nodes for them' (also s/I2P/Tor) sound somewhat sensible to me
< laanwj>
especially as I2P peers are more likely to know about other I2P peers than general ones
< laanwj>
what will also be important is to keep our I2P and TOrV3 hardcoded seeds up to date (also a 23.0 issue, for 22.0 we're fine)
< jonatack>
set it up but then no I2P peers
< laanwj>
right
< jonatack>
was planning to (maybe) propose an update to the I2P seeds before -final, as the I2P ones are evolving, a couple seem unreliable and i am seeing more choice)
<@gribble>
https://github.com/bitcoin/bitcoin/issues/22154 | Add OutputType::BECH32M and related wallet support for fetching bech32m addresses by achow101 · Pull Request #22154 · bitcoin/bitcoin · GitHub
<@gribble>
https://github.com/bitcoin/bitcoin/issues/22154 | Add OutputType::BECH32M and related wallet support for fetching bech32m addresses by achow101 · Pull Request #22154 · bitcoin/bitcoin · GitHub
<@gribble>
https://github.com/bitcoin/bitcoin/issues/21329 | descriptor wallet: Cache last hardened xpub and use in normalized descriptors by achow101 · Pull Request #21329 · bitcoin/bitcoin · GitHub
< michaelfolkson>
They are all included in feature freeze right?
< michaelfolkson>
I didn't follow when the feature freeze was extended to
< achow101>
I don't think we gave a hard deadline for feature freeze
< sipa>
feature freeze is always a bit of a fuzzy concept
< sipa>
as the boundary between bug and feature isn't always clear either
< michaelfolkson>
Ok cool
< achow101>
istm #22154 and #19651 are more bugfix-ish and can be merged after feature freeze
<@gribble>
https://github.com/bitcoin/bitcoin/issues/22154 | Add OutputType::BECH32M and related wallet support for fetching bech32m addresses by achow101 · Pull Request #22154 · bitcoin/bitcoin · GitHub
< michaelfolkson>
So priority is #22154 it appears
<@gribble>
https://github.com/bitcoin/bitcoin/issues/22154 | Add OutputType::BECH32M and related wallet support for fetching bech32m addresses by achow101 · Pull Request #22154 · bitcoin/bitcoin · GitHub
< achow101>
I would rather we focus on #22166 and #21329 for feature freeze
<@gribble>
https://github.com/bitcoin/bitcoin/issues/21329 | descriptor wallet: Cache last hardened xpub and use in normalized descriptors by achow101 · Pull Request #21329 · bitcoin/bitcoin · GitHub
<@gribble>
https://github.com/bitcoin/bitcoin/issues/22252 | policy: Trim Packages when transaction with same txid exists in mempool by glozow · Pull Request #22252 · bitcoin/bitcoin · GitHub
< ariard>
hope it'll clear up miscommunications among us, and yes we can exchange on it during tuesday's meeting :) (actually quite a resource pointer for it)
< ariard>
it might be a good weekend read for anyone interested by L2 devs making all this noise about the mempool :p