< bitcoin-git>
bitcoin/master 6690adb Tyler Chambers: Warn when binaries are built from a dirty branch.
< bitcoin-git>
bitcoin/master f3e1768 Wladimir J. van der Laan: Merge #20468: build: warn when generating man pages for binaries built fro...
< bitcoin-git>
[bitcoin] laanwj merged pull request #20468: build: warn when generating man pages for binaries built from a dirty branch (master...fix-20412) https://github.com/bitcoin/bitcoin/pull/20468
< bitcoin-git>
[bitcoin] laanwj closed pull request #20434: contrib: Parse ELF directly for symbol and security checks (master...2020_11_pixie) https://github.com/bitcoin/bitcoin/pull/20434
< bitcoin-git>
[bitcoin] laanwj reopened pull request #20434: contrib: Parse ELF directly for symbol and security checks (master...2020_11_pixie) https://github.com/bitcoin/bitcoin/pull/20434
< elichai2>
hebasto: are you going to port `std::to_array`?
< theStack>
pp
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #20588: Remove unused and confusing CTransaction constructor (master...2012-txConstructor) https://github.com/bitcoin/bitcoin/pull/20588
< bitcoin-git>
bitcoin/master fa0f415 MarcoFalke: net: Assume that SetCommonVersion is called at most once per peer
< bitcoin-git>
bitcoin/master 00f4dcd MarcoFalke: Merge #20138: net: Assume that SetCommonVersion is called at most once per...
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #20138: net: Assume that SetCommonVersion is called at most once per peer (master...2010-netVersionOnlyOnce) https://github.com/bitcoin/bitcoin/pull/20138
< bitcoin-git>
bitcoin/master fa8abdc MarcoFalke: rpc: Use FeeModes doc helper in estimatesmartfee
< bitcoin-git>
bitcoin/master 5c4911e Wladimir J. van der Laan: Merge #20568: doc: Use FeeModes doc helper in estimatesmartfee
< bitcoin-git>
[bitcoin] laanwj merged pull request #20568: doc: Use FeeModes doc helper in estimatesmartfee (master...2012-rpcDocFee) https://github.com/bitcoin/bitcoin/pull/20568
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #20589: log: Clarify that failure to read/write fee_estimates.dat is non-fatal (master...2012-logFeeest) https://github.com/bitcoin/bitcoin/pull/20589
< vasild>
sipa: wumpus: wrt sending sendaddrv2 and the difference between the code and the BIP, I opened https://github.com/bitcoin/bips/pull/1043 to change the BIP.
< vasild>
maybe changing the code to conform to the BIP also makes sense (except maybe it is too late in rc cycle)
< wumpus>
vasild: thanks, yes, that probably makes most sense
< vasild>
Lets pick a random number N between 10 and 50 and send sendaddrv2 after receiving N messages from the peer.
< wumpus>
maybe send it out of band by entangling a qubit
< vasild>
:-D
< wumpus>
in a way I like signaling the different extensions in the same way, instead of setting on a different one for each one which seems like a jumble
< wumpus>
that said this yea shouldn't have come up so late
< vasild>
wumpus: what about doing that in 0.21.1?
< wumpus>
right now, addrv2 is is no releases, so we can still change everything, once it's in 0.21.0 release, compatibility is an issu
< vasild>
0.21.0 <-> 0.21.1 would work just fine
< vasild>
I mean - it really does not matter when sendaddrv2 is being sent and anything would work
< wumpus>
that's good
< vasild>
right now bitcoin core is the only implementation that exists, I guess. I would be a problem if another implementation arrives that expects sendaddrv2 at a certain time and is upset otherwise
< vasild>
(unnecessary strict)
< vasild>
What about changing the BIP to "sendaddrv2 can be sent at any time"?
< wumpus>
I prefer being specific in the BIP
< wumpus>
and possible more lenient in the implementation
< vasild>
ok
< wumpus>
I also think that establishing this during some negotiation phase is a good idea, instead of it being possible to switch it any time during a connection
< vasild>
it just does not make sense to flip it after exchanging 47 messages
< wumpus>
it doesn't, but if it's specified in the BIP that it's not allowed then implementations don't have to take it into account, which might allow for optimizations that wouldn't be possible if it's possible to switch at any time
< wumpus>
also, it makes it possible to recognize peers that don't support addrv2 for sure, e.g. for a future time where addrv1 becomes phased out
< vasild>
right
< vasild>
actually, if we want to recognize peers that don't support addrv2 we must mandate that sendaddrv2 is sent before sending some message X, so that we would know that if we receive X from the peer without sendaddrv2 before that, then that peer does not support it
< vasild>
I guess that is an extra argument to do it before sending verack (like wtxidrelay)
< MarcoFalke>
Looks like we'll need to work on other bugfixes #20579 , so I'd also argue to move sendaddrv2 to pre-verack
< vasild>
woho! managed to connect two nodes via i2p and they both see their i2p addresses: https://bpa.st/RK3Q
< wumpus>
vasild: that's awesome! i should install i2p some time and join in the experiments
< sipa>
vasild: very nice
< vasild>
unlike tor we can see who is connecting to us, and while it is cheap to generate i2p addresses and banning i2p bad actors makes little sense (because they can generate new address) -- we are sure that the guy connecting to us has the private key that corresponds to that address
< vasild>
ie no stealing of addresses
< bitcoin-git>
[bitcoin] BitcoinTsunami opened pull request #20591: wallet, bugfix: fix ComputeTimeSmart function during rescanning process. fixes #20181 (master...fix-computetimesmart) https://github.com/bitcoin/bitcoin/pull/20591
< jonatack>
vasild: i have i2p running (and sent a patch a while back in your PR to update -netinfo for it), lmk if you want to try to connect to each other
< jnewbery>
vasild wumpus: someone should send an update to the mailing list to notify people that the BIP is being changed. I'm happy to do that, but I don't want to step on your toes if you'd prefer to do it yourselves. Let me know which you'd prefer.
< sdaftuar>
sipa: if we're going to receive a sendaddrv2 before verack, can we delay advertising our local address until after receiving verack as well? that would be nice to have in 0.20 to help with torv3 address propagation, though we may have so many obstacles on that front that perhaps not required
< sdaftuar>
the issue i've observed right now is that my torv3 node often sends its initial address advertisement before receiving a sendaddrv2, which means the initial self-advertisement is lost serialization fails, and it can be days before the bloom filter rolls over so that we can try again
< sdaftuar>
when serialization fails*
< sipa>
sdaftuar: hmm, i see no downside really, except changing more at the last minute
< sipa>
i'll open a PR; it's useful even if it doesn't go into 0.21
< sdaftuar>
thanks!
< jnewbery>
sdaftuar: I don't see any problem with sending our self-announcement after receiving the verack. Seems like a good change
< wumpus>
jnewbery: I'm ok with you doing that thanks
< bitcoin-git>
[bitcoin] jonatack opened pull request #20592: p2p: update wtxidrelay documentation per BIP339 (master...update-wtxid-documentation-per-BIP339) https://github.com/bitcoin/bitcoin/pull/20592
< jnewbery>
I think sending the getaddr should also be sent after receiving the verack
< sdaftuar>
i think it just needs to be sent after sending our own verack?
< sdaftuar>
doesn't matter whether a peer sends us a sendaddrv2 or not, i mean
< jnewbery>
Indeed, but keeping that functionality together as a block seems easiest
< sipa>
indeed; i think we should just make sure it arrives at the receiver after our potential sendadrv2
< sdaftuar>
Indeed!
< jnewbery>
ha
< wumpus>
yes
< sipa>
sdaftuar: i don't think any code changes are needed; SendMessages (which handles both local address announcement and actual sending out of the addr messages) doesn't run until fSuccessfullyConnected is true
< sipa>
which means it can only run after verack is received
< sdaftuar>
Oh, right!
< sdaftuar>
nice
< sdaftuar>
thanks for checking that
< sipa>
i started adding "if (pto->fSuccessfullyConnected) {" in a few places, and then noticed the function starts with "if (!pto->fSuccessfullyConnected) return true;"
< sdaftuar>
i kept getting confused that PushAddress doesn't actually cause an addr message to go out immediately
< sdaftuar>
so i was thinking we needed to move the code in our ::VERSION handler to wait to invoke that until after verack, but there is indeed no need to do so
< sipa>
oh wait
< sipa>
there is a PushAddress in the version handling too
< sdaftuar>
yes that's fine though
< sipa>
that needs changing
< sdaftuar>
as all it does is queue up an addr to go out later
< sdaftuar>
in SendMessages, as you point out
< sipa>
no, because PushAddress itself will filter the addr if it's not v1 compatible
< sdaftuar>
no, that happens in serialization, i'm pretty sure
< sdaftuar>
oh!
< sdaftuar>
i missed the if at the top of that function
< sdaftuar>
you are right
< sipa>
lol
< sipa>
is it even useful to do local addr announcement inside version handling?
< sipa>
the SendMessages code will do it automatically
< sdaftuar>
i had a question for you along those lines!
< sdaftuar>
it looked to me like there was special code designed to figure out which "local" address to use, that is different from AdvertiseLocal
< sdaftuar>
and i wondered whether that was important
< sdaftuar>
otherwise, it seems redundant with the initial AdvertiseLocal we do the firs ttime we run SendMessages for a peer
< sdaftuar>
(because we initialize the first local advertisement time to 0, i think)
< sipa>
there are some differences between the two
< sipa>
but the initial version-based local addr relay doesn't set m_next_local_addr_send
< sdaftuar>
right, so we invoke AdvertiseLocal immediately anyway
< sipa>
so from reading the code, i would think that most normal connections will get the local addr twice
< sdaftuar>
which might advertise a different address, but i don't know the circumstances that would happen
< sdaftuar>
no, because of the bloom filter
< sipa>
oh, it'll be deduplicated i guess
< sipa>
it also meams that addr relay of torv3 will mostly work without code changes
< sdaftuar>
the first will get squelched, and the second will work?
< sipa>
yeah
< sipa>
at least under the conditions that the SendMessages one triggers
< sdaftuar>
that does seem to be true. pretty gross though!
< sdaftuar>
that does make the change for 0.21 to move this to pre-verack to be extra good i think
< hebasto>
what if p2p protocol will have a new rule -- "during handshaking, i.e. before verack, unknown messages from peers are allowed without subsequent sender peer disconnection"; it will allow feature negotiation. Another new rule -- "node is free in its behaviour in case of receiving unknown message after verack, i.e. it can drop message and/or disconnect peer". This suggestion seem a good tradeoff with other
< hebasto>
implementations with a strong basement for future features.
< sipa>
hebasto: i think you're preaching to the choir here
< sipa>
the opposition to that idea was on the bitcoin-dev mailing list
< wumpus>
you're definitely preaching to the choire here, I haven't heard any objection to 'allowing' unknown messages at all here
< wumpus>
the title 'heuristic hex decoding' sounded really weird to me
< sipa>
wumpus: any suggestion?
< wumpus>
sipa: I don't mean I have a problem with it, it just sounded funny
< sipa>
ah, ok
< wumpus>
I guess because it was not immediately clear to me it was about transactions
< wumpus>
yes better :)
< hebasto>
sipa> Pieter Wuille hebasto: 01:13:58 the opposition to that idea was on the bitcoin-dev mailing list -- mind point out?
< sipa>
jnewbery, sdaftuar: so it seems we currently send our own address (a) once, for outbound non-blockonly connections (b) periodically for non-blockonly connections after IBD is done