< aj>
achow101: rebased the fuzzer to pull in most of the rename and the threshold part
< achow101>
aj: right now I'm trying to parameterize the period in the same way
< aj>
achow101: i'm not sure that's a good idea -- different periods make the Warning thing in validation less useful
< aj>
achow101: (for different thresholds, the RuleChangeActivatinThreshold just needs to be <= to them)
< achow101>
it's already less useful because different thresholds
< achow101>
mostly doing it for code organization reasons since a threshold doesn't make sense without what the maximum is
< achow101>
not necessarily allowing for different thresholds
< achow101>
what I'm also working towards is having all the parameters set in a constructor for VBitsDeployment since I'm not convinced we don't have an initialized variable in a (test) deployment somewhere
< achow101>
*uninitialized
< aj>
hmm
< aj>
well, the easy solution is add a check in the unit tests that the deployment periods all match the params period, and the dep thresholds are all >= the params rulechangethreshold
< achow101>
is having a fixed rule change threshold necessary? It could just search for the minimum threshold of all current deployments
< luke-jr>
achow101: I'm guessing it is (or was) for the warnings
< aj>
achow101: yes -- you still want warnings even if there are no current deployments (you're warning about things introduced in future releases of the code)
< achow101>
luke-jr: it was for all activations, in 21392 it is just for the warnings
< achow101>
aj: we always have testdummy, so when there are no active deployments, it uses testdummy's threshold
< luke-jr>
hmm
< aj>
achow101: seems nuts to me, we want a global "what's the lowest threshold any future deployment might have", why wouldn't we just have that as a field?
< achow101>
true. we should probably lower it farther though. to 75% maybe
< aj>
yep
< jeremyrubin>
I think it probably makes sense to do a lower threshold -- anything over 0 probably suggests some sort of proposed rule change?
< jeremyrubin>
We still warn any unknown vbit right?
< achow101>
too low and we warn on all the asicboost crap
< aj>
jeremyrubin: the warning is "unknown new rules activated (versionbit X)"
< luke-jr>
I have a PR open to restore the unknown bit warnings in a asicboost-friendly way btw
< luke-jr>
#15861
< gribble>
https://github.com/bitcoin/bitcoin/issues/15861 | Restore warning for individual unknown version bits, as well as unknown version schemas by luke-jr · Pull Request #15861 · bitcoin/bitcoin · GitHub
< jeremyrubin>
tbh I think that w.r.t. the threshold, the periods are probably of more concern?
< achow101>
aj: oh nice. nit: name it m_threshold (something something style guide)
< jeremyrubin>
Do we nuke all the validation caches after a soft fork activates?
< jeremyrubin>
E.g., the scriptcache?
< jeremyrubin>
Ah we commit to the validation caches for those
< bitcoin-git>
bitcoin/master fa4e088 MarcoFalke: wallet: Mark replaced tx to not be in the mempool anymore
< bitcoin-git>
bitcoin/master 6c156e4 MarcoFalke: Merge #18842: wallet: Mark replaced tx to not be in the mempool anymore
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #18842: wallet: Mark replaced tx to not be in the mempool anymore (master...2004-walletBumpFeeReplacedTxNoMempool) https://github.com/bitcoin/bitcoin/pull/18842
< vasild>
fanquake: "your browser did something unexpected" wtf!?
< vasild>
btw I was just logged out of my github account and had to enter user/pass again. This has not happened for months, if not years. May be related.
< fanquake>
I was also logged out earlier today
< bitcoin-git>
[bitcoin] jnewbery opened pull request #21395: Net processing: Remove redundant CNode.address member (master...2021-03-remove-address) https://github.com/bitcoin/bitcoin/pull/21395
< sdaftuar>
jeremyrubin | Do we nuke all the validation caches after a soft fork activates? <------- the scriptcache includes a hash of the consensus flags that a transaction's scripts were validated with, so that when a softfork activates we just end up with cache misses, and not consensus failure
< wumpus2>
re: the "extreme rare github security issue" you might want to check https://github.com/settings/security-log that nothing weird has been done with your account, it doesn't show all events (like pushes) unfortunately
< jonatack>
good idea. "Logged in - Unknown location"... goooood
< ariard>
#19160 is pretty mature but still needs more reviewers :)
< jonatack>
ariard: indeed. it's on my list (sorry for the delay)
< shefsam>
Where can I discuss a hacked Bitcoin Core Wallet? I recently opened my wallet after months if not years to find the last transaction - not mine and sweeping the entirety of my balance to an outside address I do not know.
< wumpus>
i also intend to look at the multiprocess stuff but haven't got around to it yet
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #21398: doc: Update fuzzing docs for afl-clang-lto (master...2103-docFuzzAflPlusPlus) https://github.com/bitcoin/bitcoin/pull/21398
< ariard>
right it mentipns asmap which is only one of the mitigation proposed in erebeus paper iirc
< ariard>
(but better to do feedback after meeting, let's mov on)
< jnewbery>
next topic?
< jnewbery>
#topic erlay (gleb)
< core-meetingbot>
topic: erlay (gleb)
< gleb>
Hi! Just a little update here. I was working on Erlay refactor in a separate branch, currently addressing great comments from jnewbery and ariard. Gonna finish that in a day or two.
< gleb>
jnewbery: perhaps we'll invite everyone to full review again, after you give a quick skim of the high-level encapsulation approach etc in couple days?
< gleb>
or i should just do it once im done, if you think it's good enough?
< gleb>
just trying to make review more focused and optimize efforts
< phantomcircuit>
HELLO
< jnewbery>
gleb: I've left my comments. I don't think you need to wait for another round from me before opening a PR for the new branch.
< phantomcircuit>
jnewbery, i'd like to talk about #21106 today as well
< gleb>
jnewbery: alright, thank you. that's the plan then
< gleb>
im done :)
< jnewbery>
phantomcircuit: sounds good. Thanks
< jnewbery>
any questions about erlay before the next topic?
< sipa_>
hi
< jnewbery>
#topic update on disabletx (aj)
< core-meetingbot>
topic: update on disabletx (aj)
< aj>
jnewbery's post makes sense to me -- that we can use frelay and avoid needing disabletx; but i don't understand ariard's response; and see suhas has updated the pr some more
< amiti>
+1
< jnewbery>
aj: did you read the log from the general bitcoin core irc meeting last week? There was quite a lot of discussion about using a -blocksonly node and occasionally sending txs from it
< aj>
jnewbery: i did, but as i understand it we can cope with that via fRelay fine
< jnewbery>
I hadn't thought about it any more since then, but I wasn't sure if we could cope with that via fRelay
< ariard>
aj: my point was bip37 original scope was unclear if it concerns only inv(tx) or any tx-related message, bip338 has the merit to clarify
< jnewbery>
the scenario is that you have a border node, and then a core node that only connects to that border node, and is -blocksonly. You want to be able to send txs from the core node to the border node. If we disallowed tx relay based on fRelay=false, then that wouldn't work(?)
< aj>
jnewbery: it differentiates tx's based on direction -- ie if i say fRelay=false, you won't send txs to me, but i might send the to you
< jnewbery>
txRelay isn't required for receiving txs?
< jnewbery>
if that's the case, then I think I agree with you, but need to think about it some more
< aj>
txrelay isn't required for receiving i think, just trequest.cpp
< aj>
trequest.cpp
< aj>
gah
< aj>
txrequest.cpp
< jnewbery>
aj: yes, I think you're right
< amiti>
some small changes are needed to support, but yeah, we don't need the whole txrelay struct to receive
< aj>
ariard: i think bip 37 without bip 111 (the service bit) is pretty dead? bip 60 seems to match what i described above
< jnewbery>
aj: I agree
< aj>
ariard: if you advertise the bloom service bit, then i think you just treat that as no possibility of being lightweight, and everything works fine; and obv if you don't advertise the service bit, you already kill the connection if they try doing bloom stuff to you
< jnewbery>
(I've pinged sdaftuar, since he's probably interested in this)
< sdaftuar>
hi, sorry i'm late to the discussion. i don't understand why we'd go to these contortions to avoid a new p2p message, but if someone is interested in implementing this a different way, please feel free
< aj>
sdaftuar: i think it should not end up that twisted fwiw
< ariard>
aj: bip 60 "Whether the remote peer should announce relayed transactions or not" is a bip-60 compliant client allows to send TX messages after receiving `fRelay=false`
< aj>
ariard: i don't follow what you said?
< jeremyrubin>
sdaftuar: don't we only have 8e28 possible p2p messages?
< sipa>
i think fewer p2p messages with meanings that differ slightly across versions is slightly worse than separate ones with clear well defined meaning :)
< aj>
ariard: I run a -blocksonly node. You run a bip60 compliant node. I tell you fRelay=false. You don't send me any txes. You told me fRelay=true. Therefore I can send you a tx, without violating bip60.
< ariard>
aj: that's my point the "You don't send me any txes" isn't clearly documented in the bip, a naive implementation of bip 60 could try to send raw, unannounced TX messages
< ariard>
and don't understand why the connection is severed
< sipa>
if you're going by reading the spec by the letter... what does bip60"s fRelay flag even mean for a client that chooses not.to implement bip37?
< amiti>
I'd like explicit p2p messages with clear meanings, but what I don't like about the current disabletx proposal is the implicit disabling of addrs. I understand the hesitation to want to have a better understanding of addr relay before implementing a new protocol message, but I think that having disabletx implicitly disable addrs would make it harder for us to have clear, explicit messages about relay in the
< amiti>
long-term. so that makes me more inclined to use what we have, since it suffices
< aj>
ariard: it's always the sender's choice to not send a tx?
< ariard>
aj: right but imo current bip60 semantic isn't clear with what the sender is allowed to do
< sipa>
aj: i think ariard is trying to point out that bip37/bip60 only talk about *announcing* transactions, not sending (possibly unannounced) transactions
< sipa>
amiti: i think that's fair... i also don't really know what the issue would be with a sendaddrtypes message with a bitvector of bip155 network types for example (apart from not wanting to deal with that too right now)
< aj>
sipa: ie, if I say fRelay=false, ariard shouldn't send me INVs but perhaps should reply to a GETDATA with a TX?
< ariard>
yes that's it, and sending unannounced transactions is tolerated behavior for now
< sipa>
aj: maybe, or maybe he could still send you an unannounced transaction
< aj>
ariard: hmm. i wouldnt tolerate a TX message from you if i'd connected to you as a block-relay-only peer
< jnewbery>
ok, next topic?
< aj>
anyway, should we let phantomcircuit have the floor?
< jnewbery>
thanks aj
< jnewbery>
#topic IBD latch across restarts (phantomcircuit)
< core-meetingbot>
topic: IBD latch across restarts (phantomcircuit)
< ariard>
aj: how I know I'm a block-relay-only peer ? e.g what the exact scope of "block-relay-only"?
< ariard>
but yeah let's move on
< phantomcircuit>
eh hem
< phantomcircuit>
after discussing this with a number of people sipa and sdaftuar especially i think the better approach to a network event that latches all of the listening peers into ibd is simply to make the headers sync peer selection algorithm looser at intervals since we made progress on getting out of ibd
< phantomcircuit>
that more directly addresses the issue
< sipa>
phantomcircuit: did you see sdaftuar last's response where he suspects the issue is block fetching, not header syncinf?
< phantomcircuit>
(can also serve headers when we're past the chains minimum work regardless of ibd)
< phantomcircuit>
sipa, yes, we discussed it here, he missed that compact blocks received will not trigger a getheaders when we're in ibd
< sipa>
ah
< sipa>
so what precisely do you propose to address this?
< phantomcircuit>
if we haven't made progress on headers sync in at least INTERVAL time simply request headers from all peers
< phantomcircuit>
im sure there's more nuisanced solutions that could be employed but that's the most direct to prevent the issue
< gleb>
is there a summary of the issue and this discussion anywhere? I find it difficult to follow without prior context
< jnewbery>
phantomcircuit: The scenario that you're trying to address (all or most of the listening nodes are restarted) seems like it'd always need some kind of manual intervention.
< aj>
this was the recent dogecoin network failure more or less?
< emzy>
I think so.
< phantomcircuit>
jnewbery, sure, lets say there's a crash bug, which has existed in the past
< sipa>
aj: yes
< phantomcircuit>
i'll note that what i describe in the pr has happened on an altcoin derived from the bitcoin codebase and is relatively recently backported so it's not ancient nonsense
< sipa>
0.14 iirc
< aj>
phantomcircuit: (nuanced, not nuisanced :)
< aj>
header polling sure sounds better than perma-latching !IBD to me?
< sipa>
phantomcircuit: given that the dont-ask-headers-from-everyone is purely a bandwidth-preservation heuristic, i think it makes perfect sense to disable/weaken it in cases where we're worrying about being unable to sync
< jnewbery>
it's really difficult to follow the PR to be honest. I
< jnewbery>
I'm not surprised that people are confused
< sipa>
ignore the code in the PR, it's just the discussion there that matters
< jnewbery>
there seem to have been several different approaches, and then comments left and later deleted
< sipa>
the code will be very different once it's hammered out
< sipa>
phantomcircuit, sdaftuar: would it be possible to just post a summary/update of your understanding of what should be addressed?
< jnewbery>
that seems like a sensible next step
< sipa>
because it seems there has been some progress on IRC that's not mentioned there
< phantomcircuit>
aj, English is hard
< aj>
phantomcircuit: nuance is often a nuisance, to be fair
< sipa>
but in any case, if there's reasonable evidence that all we need to do is making headers syncing less restrictive, i think weakening just that in case we haven't found new headers in a while seems like a reasonable approach
< phantomcircuit>
sipa, it's a little confusing, since there's a bunch of things that are potentially separately contributing to the issue
< sipa>
aj: that's a pretty unnuanced statement
< jnewbery>
ok, anyone have any final updates before we close the meeting?
< phantomcircuit>
sipa, i think relaxing the headers sync peer selection combined with serving headers if we're past minwork would get us a long way, but we ban peers for sending headers which are below out minwork and the minwork goes up overtime, so old peers that are restarted could get banned
< sipa>
phantomcircuit: but the banning is only for headers pre-last-checkpoint, which is ancient
< sipa>
and if clients are introduce a new checkpoint that'd need huge scrutiny anyway
< sipa>
so maybe that's not crazy to assume that any minpeerwork is going to be past any-reasonable-client's checkpoints
< bitcoin-git>
[bitcoin] hebasto opened pull request #21400: build, qt: Fix regression introduced in #21363 (master...210309-regression) https://github.com/bitcoin/bitcoin/pull/21400
< aj>
ariard: you have no way of knowing you're a block-relay-only peer, apart from me setting fRelay=false
< aj>
ariard: (which could also mean i'm running with -blocksonly=true and, if you sent NODE_BLOOM, that i want to do bloom stuf later)
< phantomcircuit>
sipa, right so that's less dangerous
< sdaftuar>
sorry i missed most of the meeting, had to step away
< sdaftuar>
phantomcircuit: headers sync is insufficient to solve the problem
< sdaftuar>
the biggest issue is that we don't call FindNextBlocksToDownload on inbound peers if we have outbound peers and we're in IBD
< sdaftuar>
if we relaxed that, i believe it would completely solve the problem. the issue you spotted with compact blocks shoulnd't be relevant, because we don't send a peer a block announcement via compact block unless they ask for it
< sdaftuar>
and we don't turn on compact block announcements on any of our peers until we're out of IBD
< sdaftuar>
that said, we could also loosen up headers sync with inbound peers anyway (in my model of how we get out of a situation like this, you have to wait for a block to be found; by syncing with inbound peers more aggressively than we do now you could shorten that window)
< sdaftuar>
but it's not enough to just treat this as a headers sync issue -- we have to fix the block fetching
< lightlike>
aj: "you have no way of knowing you're a block-relay-only peer (...)" - wouldn't we still send FEEFILTER while running -blocksonly=true, but never on a block-relay-only connection (because m_tx_relay==null)?
< phantomcircuit>
sdaftuar, that's probably *also* something that's wrong, but what i see is that you can't get the headers first anyways
< sdaftuar>
phantomcircuit: do you have a mechanism for how a compact block could be delivered when headers haven't yet synced?
< sdaftuar>
(and while the node is in ibd)
< phantomcircuit>
sdaftuar, i meant the headers sync is broken in the other way
< phantomcircuit>
this is why it's so confusing, there's a bunch of different things which when in ibd might be the issue, but the only one i could observe is that no peer i could connect to and asked would give me headers
< sdaftuar>
my understanding is that at some point after an inbound peer connects to you (while you're in ibd), it should announce a new block
< sdaftuar>
at that point, that announcement will have to be via INV, and it will trigger headers sync
< sdaftuar>
however, due to the block fetching being broken as i described before, you'll be in the stuck state you observed
< sdaftuar>
i can't see a way for compact block relay to even start until you leave IBD so i think that's a completely separate issue
< sdaftuar>
of course if no blocks are found after that peer connects, then indeed you would never even get the chain they have
< sdaftuar>
but i don't think that's the root issue here? you're concerned with inbound peers being your only bridge to the honest network, and making sure you eventually sync, i think
< phantomcircuit>
sdaftuar, right the gets to the other issue that i forgot about, we don't announce our address while in ibd, so starting a new listening peer you won't get inbound connections anyways
< phantomcircuit>
i forgot about that one
< sdaftuar>
don't we immediately push our address to our outbound peers when we initiate a connection, right at the end of ver/verack?
< sdaftuar>
oh, that is gated on IBD, i see
< sdaftuar>
still i don't think that's really a relevant concern here. we're just trying to make block sync work from an inbound peer i think?
< sdaftuar>
we could probably get rid of the addr relay gating stuff. i think the motivation is that we don't want peers to connect to us if we can't help them sync blocks, but given that good addr propagation takes much longer than block sync should, we should probably just announce our address always i think
< sdaftuar>
i think a long time ago a peer that never responded to headers and never provided blocks could dos a bitcoin core node, but now that we time out an outbound peer that doesn't sync headers with us when we're looking for headers, this shouldn't be a big concern
< bitcoin-git>
[bitcoin] achow101 opened pull request #21401: Refactor versionbits deployments to avoid potential uninitialized variables (master...encap-vbits-params) https://github.com/bitcoin/bitcoin/pull/21401
< phantomcircuit>
sdaftuar, possibly? im not sure if that was eclipse related maybe sipa knows
< phantomcircuit>
sdaftuar, for sure you won't get any inbound until you have finished ibd though
< sdaftuar>
phantomcircuit: have i managed to convince you that block fetching is all that we need to fix? if you have some scenario in mind where that would be insufficient, i'd like to try to understand what that is