< aj>
sipa: sorry 35 not 100; if you get less than that in a single message, it means you've emptied setInventoryTxToSend
< sipa>
right
< aj>
sipa: which brings you back to knowing about a timepoint, but the timepoint's chosen by the victim's poisson schedule, not the attacker's choice of when to connect?
< phantomcircuit>
promag, it depends on which rpc calls you're making
< promag>
how so?
< phantomcircuit>
if you're making lots of fast calls it will matter, if you make a few slow calls you won't even notice
< phantomcircuit>
promag, yeah the getblockhash rpc is probably the biggest win for batching, ages ago i implemented a getblockhashes that took a range but it was too craptastic for a pr
< aj>
sipa: yes, it does
< promag>
phantomcircuit: right
< sipa>
aj: so WDYT about relay pool 2 minutes, bloom filter per peer for last (whatever can be relayed in 2 minutes) transactions, things can be requested if they're in the bloom filter OR in mempool and older than 2 minutes OR older than the last BIP35 response; BIP35 responses don't go into the bloom filter
< aj>
oh, i see, bitcoin does 7tx/second because that's 420tx/minute -- blazing fast
< sipa>
?
< aj>
"420 blaze it" nevermind
< phantomcircuit>
ok so now i have a keypool problem, i've generated a list of scripPubKeys and can get notified when the keypool changes... but this is in a tight loop and im not sure if the NotifyCanGetAddressesChanged thing is guaranteed to run before anything else
< phantomcircuit>
also it's kind of ugly to use a callback here
< aj>
sipa: why not do bip35 via bloom as well?
< sipa>
aj: it would easily overflow the filter
< aj>
right, right
< sipa>
unless the filter is made much larger
< aj>
yeah, i was thinking of making it so bip35 wouldn't let you see stuff that hadn't made it into the bloom filter, but seems too complicated, esp if we think bip35 is only for trusted-ish peers
< sipa>
you have to explicitly enable bip37, or whitelist the peer
< aj>
so "whatever can be relayed in 2 minutes" ~= 840 tx for inbound, 2110 tx for outbound? i think it'd make sense to be "bloom filter (for whatever can be relayed in 1.5*X minutes) OR in the mempool and older than X minutes" so there's no weird, exploitable gaps?
< sipa>
hmm, what kind of gaps?
< sipa>
the rolling bloom filter is already maintaining between 1.5*x and x entries
< aj>
sipa: as in txs drop out of the bloom filter in <2m, because your poisson timing goes really quick by random chance?
< aj>
i guess there's 24 intervals in 2minutes, so 1.5x is probably vast overkill
< sipa>
ah!
< sipa>
this calls for some statistics
< aj>
(when you said "ah!" i thought, "oh no, a spider??", now i wish it were just that)
< sipa>
so this is a poisson distribution with lambda=24 (we expect 24 inv broadcasts in 2 minutes)... there could be up to 50 broadcasts however in 2 minutes, with probability up to 99.9999%
< aj>
lambda=60 for outbounds, i guess?
< sipa>
yup, and there we need to account for up to 95 broadcasts in 2 minutes
< sipa>
(to have 99.9999% probability)
< aj>
means about 1800 and 3500 txs for 9.7kB and 19kB filters?
< sipa>
more accurate numbers: 51 and 100
< midnight>
har har "blazing fast"
< aj>
sipa: 52 would stay just under 10kB; 105 would be just under 20kB. so about 1.3M in total with max peers?
< sipa>
i think anything in the order of 10s of kilobytes per peer is negligable
< aj>
yeah
< sipa>
could just go for an even 3500 sized filter for everyone
< hebasto>
MarcoFalke: how to preserve travis log with an intermittent error for issue submitting? If I re-run build log pointed by url will be re-written...
< cncr04s>
can any one explain this "[00:47:45] cncr04s #bitcoin-core-dev Cannot change nickname while banned/quieted on channel" I have not spoken on this channel in weeks so...
< gwillen>
cncr04s: assuming someone doesn't come correct me, and given that you appear not to be banned or quieted now, I would assume it was an accident / collateral damage of some other ban
< phantomcircuit>
ok i don't get it how am i supposed to use NotifyCanGetAddressesChanged ?
< phantomcircuit>
BlueMatt, halp
< achow101>
phantomcircuit: what are you trying to use it for?
< achow101>
and how are you trying to use it?
< provoostenator>
I can also gurantee Blue Matt will not know the answer, unless he's a secret wallet code admirer :-)
< BlueMatt>
right, that.
< phantomcircuit>
BlueMatt, hi
< BlueMatt>
sup.
< phantomcircuit>
achow101, im just trying to set a flag in CWallet to recalculate a list of script pubkeys for block filter index based rescanning
< bitcoin-git>
[bitcoin] practicalswift opened pull request #19143: tests: Add fuzzing harnesses for CAutoFile, CBufferedFile, LoadExternalBlockFile and other FILE* consumers (master...fuzzers-FILE) https://github.com/bitcoin/bitcoin/pull/19143
< achow101>
phantomcircuit: if the list is persistent, can't you just add to it as new scriptpubkeys are generated?
< phantomcircuit>
achow101, there's a bunch of ways for the list to be changed now
< achow101>
NotifyCanGetAddressesChanged does not necessarily mean that a new scriptPubKey has been generated. it's used to indicate whether it's possible to even get a new address
< achow101>
phantomcircuit: where?
< sipa>
i think it's pretty much impossible to just incrementally update that list
< sipa>
just inserting a new pubkey may cause some indirectly multisig combination with it inside p2sh/p2wsh to become marked IsMine
< achow101>
but also why is the list persistent? I thought we had discussed previously that you should compute it at rescan time
< provoostenator>
If someone picks up #15845 it should be significiantly less painful to just rescan the whole wallet history
< achow101>
provoostenator: I think that's what phantomcircuit is doing
< provoostenator>
Ah OK, that's good news
< sipa>
achow101: i think the problem is that during rescan, new keys may be pulled from the keypool, which would require recomputing the list
< provoostenator>
That was one of the bugs in the original implementation, at some point anyway, not sure if it was fixed.
< achow101>
lock cs_wallet *ducks*
< phantomcircuit>
provoostenator, that's what im doing and why im asking about it
< sipa>
"this new rescan is super fast; one minor downside is that it deadlocks whenever it finds a new tx"
< provoostenator>
Or just start the rescan from scratch if you had to expand the keypool :-)
< provoostenator>
"this rescan runs in circles, but it does so really fast"
< phantomcircuit>
really i just need a flag that says the keypool has been changed, but it seems like there's multiple places that can happen now so this callback is the thing to use
< achow101>
provoostenator: and I think that brings us back to the phantomcircuit's original question about NotifyCanGetAddressesChanged as he needs a way to know whether to restart if the keypool changed
< achow101>
phantomcircuit: NotifyCanGetAddressesChanged is a boost signal. you have to bind and connect to it from elsewhere
< phantomcircuit>
it's a boost signal for whatever the current pwallet is i guess? it has no parameters
< achow101>
yes
< achow101>
it's a CWallet member
< achow101>
(well ScriptPubKeyMan member too, but these end up in CWallet)
< provoostenator>
Would it makes sense to have a skpman->keypool_update_at field?
< phantomcircuit>
provoostenator, i was going to use an atomic bool flag since it doesn't really matter when it was updated just that it needs to be regenerated
< phantomcircuit>
achow101, so can i use this-> within the callback? it doesn't seem like i can
< achow101>
why wouldn't you be able to?
< sipa>
boost callbacks happen in the same thread
< sipa>
maybe that's what phantomcircuit is confused about?
< phantomcircuit>
oh right i was trying to use a lambda and i had the syntax very wrong
< phantomcircuit>
sipa, the false positive rate for this seems really high, searching only for p2pkh on 12k keys none of which have actually been used im seeing a pretty consistent scroll of blocks matching
< phantomcircuit>
"Empirical analysis also shows that was chosen as these parameters minimize the bandwidth utilized, considering both the expected number of blocks downloaded due to false positives and the size of the filters themselves." - BIP158
< phantomcircuit>
that must only be true for pretty small filters
< achow101>
phantomcircuit: are you sure there isn't a bug?
< bitcoin-git>
[bitcoin] fjahr opened pull request #19145: [WIP] Allow hash_type options for gettxoutsetinfo (master...csi-3-muhash-rpc) https://github.com/bitcoin/bitcoin/pull/19145
< phantomcircuit>
achow101, pretty sure, it successfully finds all the blocks that do have keys in a regtest example and mostly doesn't false positive since each block only has a coinbase, im now benchmarking against the main chain and to say there's a lot of false positives would be quite the understatement
< phantomcircuit>
achow101, i just updated #19116 with the exact code im running
< sipa>
phantomcircuit: i think bip158 was designed for much smaller sets of keys, yes
< sipa>
*sets of sPKs
< sipa>
it has an fprate of 1/784000
< sipa>
so if you're matching with a set of 120000 sPKs you'd see a match for 120000/784000 = 1.5% of blocks
< sipa>
i expect that's still a lot better than reading through all blocks
< sipa>
eh, 12000 sPKs
< phantomcircuit>
it took 53m using the block filters, currently running without to compare
< phantomcircuit>
with them it's about equal time between the filter operations and sha256 which i assume is verifying merkle tree roots
< phantomcircuit>
possibly saving a hash of the entire block in the block index would be a good idea?
< achow101>
A default legacy wallet has at least 8000 sPKs
< achow101>
so 12000 sPKs is not unreasonable
< phantomcircuit>
yeah and im doing this cause i know there's wallets with way more than that
< phantomcircuit>
i have a testnet wallet with like a million keys, most of which were actually used
< sipa>
with a million keys the bip158 filter won't help
< achow101>
phantomcircuit: that sounds like it'd match on the entire testnet chain with a pretty high true positive rate too
< sipa>
you'd match every block
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #19146: doc: Remove release note fragments of 0.20.1 release, Add release-notes-0.20.0 (master...2006-docRel) https://github.com/bitcoin/bitcoin/pull/19146
< phantomcircuit>
sipa, i guess i could add another index with different parameters?
< phantomcircuit>
the size of the filter is pretty irrelevant if you're not sending it to anybody
< phantomcircuit>
and im only hashing the wallet contents per block not the block contents
< sipa>
phantomcircuit: maybe... but is that worth it? i expect that the number of wallets with million of keys is tiny, and when they need a rescan, it's typically a one time thing
< luke-jr>
fwiw, I added pbzx to my Gentoo overlay
< phantomcircuit>
sipa, maybe just as an optional extra index? not something for everybody to be generating for sure
< phantomcircuit>
yeah without the filter was actually faster
< phantomcircuit>
46m vs 53m
< bitcoin-git>
[bitcoin] ariard opened pull request #19147: Document BanMan with regards to malicious exploitation (master...2020-06-doc-banman-infra) https://github.com/bitcoin/bitcoin/pull/19147
< phantomcircuit>
ok so the 1 in a billion filter might be too extreme