--- Day changed Wed Nov 06 2019
01:00 < molz> hi guys, is the meeting going to start at 17:00 UTC today?
01:28 -!- willcl_ark [~quassel@cpc123762-trow7-2-0-cust7.18-1.cable.virginm.net] has quit [Ping timeout: 264 seconds]     
01:44 -!- willcl_ark [~quassel@cpc123762-trow7-2-0-cust7.18-1.cable.virginm.net] has joined #bitcoin-core-pr-reviews     
02:31 -!- jonatack [~jon@213.152.162.5] has joined #bitcoin-core-pr-reviews     
04:22 -!- diogosergio [~diogoserg@212.36.34.126] has joined #bitcoin-core-pr-reviews     
05:07 -!- jonatack [~jon@213.152.162.5] has quit [Ping timeout: 240 seconds]     
05:13 -!- vindard [~quassel@190.83.165.233] has quit [Remote host closed the connection]     
05:13 -!- vindard [~vindard@190.83.165.233] has joined #bitcoin-core-pr-reviews     
05:15 -!- vindard [~vindard@190.83.165.233] has quit [Remote host closed the connection]     
05:17 -!- vindard [~vindard@190.83.165.233] has joined #bitcoin-core-pr-reviews     
05:30 -!- vindard [~vindard@190.83.165.233] has quit [Remote host closed the connection]     
05:30 -!- vindard [~vindard@190.83.165.233] has joined #bitcoin-core-pr-reviews     
05:30 -!- diogosergio [~diogoserg@212.36.34.126] has quit [Remote host closed the connection]     
06:29 -!- jonatack [~jon@2a01:e35:8aba:8220:6627:dad:d967:649d] has joined #bitcoin-core-pr-reviews     
06:36 -!- pinheadmz [~matthewzi@185.217.69.139] has joined #bitcoin-core-pr-reviews     
07:15 -!- openoms [~quassel@cpc115066-stok20-2-0-cust313.1-4.cable.virginm.net] has quit [Ping timeout: 265 seconds]     
07:15 -!- openoms_ [~quassel@cpc115066-stok20-2-0-cust313.1-4.cable.virginm.net] has joined #bitcoin-core-pr-reviews     
07:21 <@jnewbery> molz: yes, meeting is always at 17:00 UTC
07:22 <@jnewbery> so if you're in N America, it'll be an hour earlier than you're used to because of daylight savings ending
07:22 <@jnewbery> we'll get started in just over an hour and a half
07:22 <@jnewbery> Notes and questions: https://bitcoincore.reviews/15845.html
07:37 < molz> jnewbery, thank you :)
07:46 -!- openoms_ [~quassel@cpc115066-stok20-2-0-cust313.1-4.cable.virginm.net] has quit [Ping timeout: 240 seconds]     
07:49 -!- soju [~soju@2601:640:8780:6d90:eca0:7eb0:4e45:8ac7] has joined #bitcoin-core-pr-reviews     
08:10 -!- pinheadmz [~matthewzi@185.217.69.139] has quit [Quit: pinheadmz]     
08:18 -!- lightlike [~lightlike@2001:16b8:577c:d900:11d3:b4a1:4c35:e2b8] has joined #bitcoin-core-pr-reviews     
08:35 -!- ysangkok [janus@hapy.0x90.dk] has joined #bitcoin-core-pr-reviews     
08:39 -!- michaelfolkson [~textual@62.60.63.228] has joined #bitcoin-core-pr-reviews     
08:50 -!- diogosergio [~diogoserg@212.36.34.126] has joined #bitcoin-core-pr-reviews     
08:53 -!- shesek [~shesek@185.3.145.80] has joined #bitcoin-core-pr-reviews     
08:53 -!- shesek [~shesek@185.3.145.80] has quit [Changing host]     
08:53 -!- shesek [~shesek@unaffiliated/shesek] has joined #bitcoin-core-pr-reviews     
08:54 -!- sebastianvstaa [~sebastian@HSI-KBW-091-089-090-072.hsi2.kabelbw.de] has joined #bitcoin-core-pr-reviews     
08:55 <@jnewbery> 5 minute warning!
08:56 -!- dho [~dho@freenode/staff/dho] has left #bitcoin-core-pr-reviews []     
08:56 -!- emilengler_ [~emilengle@unaffiliated/emilengler] has quit [Quit: ZNC 1.7.2+deb3 - https://znc.in]     
08:58 -!- jkczyz_ [~jkczyz@135.84.132.56] has joined #bitcoin-core-pr-reviews     
09:00 <@jnewbery> ding ding ding
09:00 <@jnewbery> We don't have a meetbot, but I'm going to use these tags to make grepping my logs easier:
09:00 <@jnewbery> #startmeeting
09:00 <@jnewbery> hi!     
09:00 < jonatack> hi     
09:00 -!- pinheadmz [~pinheadmz@172.98.82.141] has joined #bitcoin-core-pr-reviews     
09:00 < amiti> hi     
09:00 < pinheadmz> Hi!     
09:00 <@jnewbery> Hi all. This week's PR is 15845: "Fast rescan with BIP157 block filters". Notes and questions at https://bitcoincore.reviews/15845.html.
09:00 <@jnewbery> Before we start, I wanted to give a couple of tips for using IRC here.
09:01 <@jnewbery> First: jump right in! I'll prompt the conversation with the questions at https://bitcoincore.reviews/15845.html, but don't feel like you need to wait until after those questions to make your comment or ask your question.
09:01 < lightlike> hi     
09:01 < michaelfolkson> Hey     
09:01 < jonatack> #topic irc tips :p
09:01 <@jnewbery> You're not being rude if I ask a question and you make an unrelated comment!
09:01 <@jnewbery> We can have multiple threads going at the same time. If it gets too confusing I might step in and ask people to address one topic first, but in general just go for it.
09:02 <@jnewbery> Second: don't ask to ask questions. There's no need to say things like "I have a question that I think might be off topic" or "is it ok if I ask my question now?". Just jump right in and ask. If it's off-topic, then people will tell you!
09:02 <@jnewbery> ok, that's it for now. Let's get started!
09:02 -!- emilengler [~emilengle@unaffiliated/emilengler] has joined #bitcoin-core-pr-reviews     
09:02 <@jnewbery> What did everyone think of the PR?
09:02 < provoostenator> (hi)
09:02 <@jnewbery>  Concept ACK, approach ACK, tested ACK, or NACK?
09:02 < provoostenator> It's fast!
09:03 < provoostenator> It's increased my sanity :-)
09:03 < pinheadmz> Conceptually very cool use case and makes a lot of sense. Rescanning is terrible and this is a huge improvement
09:03 < jonatack> Tested ACK review underway.
09:03 < jkczyz_> hi     
09:04 <@jnewbery> great! Everyone seems happy with the concept at least. What did you all do to review and test?
09:04 < pinheadmz> Also reveals how a full neutrino client would work
09:04 < provoostenator> I mostly tested it by scanning old wallets.
09:04 < pinheadmz> Just read the code. Didn't have time to install before the meeting
09:04 < provoostenator> Also briefly read the code
09:05 <@jnewbery> did people have a chance to read BIP 158? It's not as scary as you might think
09:05 < jkczyz_> yep     
09:05 < jonatack> Built, ran all tests, read code, now poking through the tests, would want to test against wallets too.
09:06 < pinheadmz> Yes and we merged bip158 filters into bcoin recently as well
09:06 -!- Talkless [~Talkless@hst-227-49.splius.lt] has joined #bitcoin-core-pr-reviews     
09:06 -!- soju [~soju@2601:640:8780:6d90:eca0:7eb0:4e45:8ac7] has quit [Remote host closed the connection]     
09:07 <@jnewbery> pinheadmz: great. Are you serving compact block filters over p2p yet?
09:07 < pinheadmz> No :-)
09:07 < pinheadmz> Just indexing and rpc
09:07 < pinheadmz> P2P service is coming
09:07 < emilengler> Hi     
09:07 <@jnewbery> Was anyone able to answer q3: How many scripts will a newly created wallet have? How many items will be added to the GCS returned by GetAllRelevantScriptPubKeys()?
09:07 < molz> I compiled PR 15845 for windows and i'm running it now but i don't see anything different, do I need to put something in bitcoin.conf?
09:08 <@jnewbery> molz: you'll need to use -blockfilterindex=1 to build the index
09:08 < provoostenator> FYI: p2p is here: https://github.com/bitcoin/bitcoin/pull/16442
09:08 < molz> jnewbery, ah i see, thanks for that tip
09:08 < MarcoFalke> molz: There is no user facing change (other than less time spent rescanning)
09:09 <@jnewbery> Thanks provoostenator. I think we should cover that in a future PR review club. Anyone want to host that one? :)
09:09 < molz> im so used to running btcd where i don't need to put any flag for the filters in .conf, thanks
09:09 < provoostenator> Yeah, that's a fun one, and you can test it against btcd (which I did, and found some issues) and other clients.
09:09 < sebastianvstaa> jnewberry: how is -blockfilterindex=! different from -rescan?
09:10 <@jnewbery> blockfilterindex=1 builds the index in the background. -rescan=1 rescans the blockchain for all wallet transactions on startup
09:10 < sebastianvstaa> *blockfilterindex=1
09:10 < sebastianvstaa> ok     
09:10 < provoostenator> You can also rescan using RPC, e.g. rescanblockchain FROMHEIGHT
09:11 <@jnewbery> there should be no user-facing difference running with -rescan before and after this change, except it should be _a lot_ faster after this PR
09:11 < molz> so if you didn't start the node with -blockfilterindex=1, do you have to delete 'blocks' and 'chainstate' and start over?
09:11 < sebastianvstaa> yes. on testnet it'S only 10 seconds or so now.
09:11 < provoostenator> molz: no
09:11 < molz> oh ok, provoostenator
09:11 < provoostenator> There's a new indexing system, thanks to Jimpo,
09:12 <@jnewbery> The code changes in this PR are quite small, but I think you need to understand keypools and how the wallet rescans to give it a good review
09:12 < provoostenator> Which lets you add and remove indexes without having to do a chainstate / block reindex.
09:12 < provoostenator> You can even temporarliy turn an index off.
09:13 < provoostenator> My biggest worry would be that somehow we miss some weird transaction type during the rescan. But that would (?) imply a bug in the filter definitions.
09:14 < MarcoFalke> provoostenator: Or a bug in GetAllRelevantScriptPubKeys
09:14 < Talkless> hi     
09:14 < Talkless> "This PR changes the rescan logic to first compute a Golomb-Coded Set of all the wallet’s scripts, and then tests for matches against all blocks in the block chain. Only blocks with a positive match need to be fetched and scanned by the wallet."
09:14 < Talkless> does that mean bitcoin core wallet will use these filters by itself?
09:14 <@jnewbery> provoostenator: yes, the thing that reviewers should be asking themselves is "could rescan miss a wallet transaction with this new code"
09:14 < provoostenator> Talkless: what do you mean by "by itself"?
09:14 < Talkless> not via peer services by other wallets
09:15 < MarcoFalke> Talkless: No, the filters are handled by the node, which computes the result and hands it to the wallet
09:15 < provoostenator> Talkless: correct: the node generates the filters by scanning the chain.
09:15 <@jnewbery> Talkless: There are no p2p changes in this PR
09:15 < Talkless> jnewbery: I uderstand
09:16 < pinheadmz> Did bloom filters include OPRETURN data? I think bip158 does not right?
09:16 < MarcoFalke> Talkless: I think "node" might be a better word for network connections, not "wallet"
09:16 < Talkless> but the intention is that these filters will be used only by "other" external wallets via p2p, or by bitcoin core's wallet itself/
09:16 <@jnewbery> The wallet keypool is defined here: https://github.com/bitcoin/bitcoin/blob/976cc766c42889b6d1042e4cda39045d5001f406/src/wallet/scriptpubkeyman.h#L86
09:16 <@jnewbery> (moved from wallet.h very recently)
09:16 < Talkless> "This PR changes the rescan logic to first compute a Golomb-Coded Set of all the wallet’s scripts"
09:16 < Talkless> for ALL WALLET'S SRIPTS?
09:16 < Talkless> for the wallet that resided in this current node?
09:17 <@jnewbery> That class has a very full comment
09:17 < MarcoFalke> Talkless: for the wallet that is loaded currently
09:17 < MarcoFalke> The scripts are different for each wallet (in general)
09:17 < Talkless> MarcoFalke: ok so the node will use these filters for active wallets in that node? That's what I wanted to make clear.
09:18 <@jnewbery> This part is important: "The keypool is used to implement a 'gap limit'. The keypool maintains a set of keys (by default 1000) ahead of the last used key and scans for the addresses of those keys."
09:18 <@jnewbery> so how many scripts does a newly created wallet have?
09:18 < pinheadmz> Sounds like 1000 ;-)
09:18 < pinheadmz> Oh unless we also add nested scripts
09:18 < molz> 6000? :D
09:19 <@jnewbery> molz: Yes!
09:19 < provoostenator> Keypool is not the only thing we care about. There can also be watch-only things. But not OP_RETURN afaik.
09:19 <@jnewbery> https://github.com/bitcoin/bitcoin/pull/15845#issuecomment-485167872
09:19 < molz> i cheat i cheat, i read what sipa said lol
09:19 <@jnewbery> reading what sipa says is generally encouraged
09:20 <@jnewbery> ok, so when we rescan an empty wallet, we're initially looking out for 6000 different scripts
09:20 <@jnewbery> in the current code, what happens when we match one of those scripts?
09:22 < jkczyz_> We lookup the block to see if it's really there
09:22 < provoostenator> 6000 scripts leads to a lot of false positives.
09:22 <@jnewbery> jkczyz_: I think you're referring to what happens after this PR
09:23 <@jnewbery> I meant in the master (pre-PR) code
09:23 < jkczyz_> haha, yeah I guess that's what you meant by currently :P
09:23 <@jnewbery> sorry - my question was unclear
09:23 <@jnewbery> heres the code in master: https://github.com/bitcoin/bitcoin/blob/976cc766c42889b6d1042e4cda39045d5001f406/src/wallet/wallet.cpp#L1575
09:24 <@jnewbery> SyncTransaction() is where the magic happens: https://github.com/bitcoin/bitcoin/blob/976cc766c42889b6d1042e4cda39045d5001f406/src/wallet/wallet.cpp#L1628
09:25 <@jnewbery> Does anyone want to guess? :)
09:25 <@jnewbery> what do you think we want to do when we find a matching scriptPubKey for a key that is in our keypool in a block transaction?
09:26 < jkczyz_> update our balance
09:26 < pinheadmz> Yeah add the utxo to wallet db
09:26 <@jnewbery> jkczyz_: good guess! We actually calculate balance dynamically whenever it's needed. We don't keep a running total
09:26 <@jnewbery> pinheadmz: yes!
09:27 < pinheadmz> (Or remove if we were spending!)
09:27 < ajonas> AddToWalletIfInvolvingMe
09:27 <@jnewbery> ajonas: \o/
09:27 <@jnewbery> ok, so we see a transaction that we're interested in, and then add it to the wallet
09:28 <@jnewbery> what else do we need to do? Think about the keypool
09:28 < amiti> mark the keys as used
09:28 < amiti> aka remove from keypool
09:28 < pinheadmz> And generate one to replace?
09:28 <@jnewbery> pinheadmz: we don't really 'remove' transactions from the wallet. If they send to us or spend from us, then we want to keep track of them
09:29 <@jnewbery> amiti and pinheadmz: yes and yes :) :)
09:29 < pinheadmz> So does this actually mean that the lookahead is 1000?
09:29 < pinheadmz> In the context of bip44
09:29 <@jnewbery> pinheadmz: correct
09:29 < molz> the keypool would be smaller until we use up 1000 keys then the keypool would generate another 1000?
09:29 < pinheadmz> Interesting. The bip only specifies 20 I think
09:29 < provoostenator> 1000 receive + 1000 change I think
09:30 <@jnewbery> pinheadmz: see earlier comment: "The keypool is used to implement a 'gap limit'. The keypool maintains a set of keys (by default 1000) ahead of the last used key and scans for the addresses of those keys."
09:30 < provoostenator> BIP 44 specifies a GAP limit of 20, but we don't use BIP 44.
09:30 < pinheadmz> I see
09:30 <@jnewbery> molz: no, we top up as we go. So if we drop to 999, we'll generate another 1
09:30 <@jnewbery> as long as the wallet is unlocked (not encrypted)
09:30 < ajonas> why do we need so many?
09:30 < provoostenator> With descriptor wallets it'll be 6000: 1000 for each address type, adn then receve+ change
09:31 < provoostenator> ajonas: I think it's historical, before we had HD wallets it would pre-generate private keys
09:31 < MarcoFalke> ajonas: I think it was bumped when we switched to hd wallets
09:31 < jonatack> To avoid loss of funds iirc
09:31 < provoostenator> I started with 100 I think: https://github.com/bitcoin/bitcoin/commit/4323bfeafda4a5e0101710d94b518d41819a2325
09:32 -!- b10c [~Thunderbi@i577BC684.versanet.de] has joined #bitcoin-core-pr-reviews     
09:32 <@jnewbery> sorry, was just looking at the code, trying to find where we top up the key pool
09:32 <@jnewbery> it's changed around recently
09:32 <@jnewbery> here we are: https://github.com/bitcoin/bitcoin/blob/976cc766c42889b6d1042e4cda39045d5001f406/src/wallet/wallet.cpp#L885
09:33 <@jnewbery> in AddToWalletIfInvolvingMe(), we'll mark the keypool keys as used there ^
09:33 < provoostenator> And bumbed to 1000 here: https://github.com/bitcoin/bitcoin/commit/41dc1635878766e4a810e6a7c57637d079fced64 (indeed after HD)
09:33 <@jnewbery> And MarkUnusedAddresses() calls TopUp(): https://github.com/bitcoin/bitcoin/blob/976cc766c42889b6d1042e4cda39045d5001f406/src/wallet/scriptpubkeyman.cpp#L283
09:33 <@jnewbery> L294 in that function
09:36 <@jnewbery> ok, so let's recap: a freshly created wallet will have 1000 keys (which equates to 6000 scriptPubKeys). In master, on rescan, we'll iterate through the blocks in the block chain. If we find a transaction that matches one of those scriptPubKeys, we'll mark that keypool entry (and every keypool entry before it) as used...
09:36 <@jnewbery> ... and then topup the keypool before continuing to scan the blockchain
09:36 <@jnewbery> so, if keys are used in order then we won't miss any transactions
09:37 <@jnewbery> the gap limit of 1000 means that if there are any gaps, or the keys are used out-of-order, then as long as the gap is less than 1000, we won't miss any transactions
09:37 <@jnewbery> slight correction - we have 2000 keys and 6000 scriptPubKeys (1000 external keys for handing out as addresses, 1000 internal keys for change)
09:38 <@jnewbery> any questions about any of that ^ ?
09:38 -!- zenogais [~user@cpe-76-175-74-114.socal.res.rr.com] has joined #bitcoin-core-pr-reviews     
09:38 < pinheadmz> 👍
09:39 < amiti> does the ordering / gap limit only apply to the keys handed out as addresses?
09:39 <@jnewbery> amiti: good question! You're asking because you have complete control over change keys so you wouldn't leave gaps?
09:39 < amiti> yeah
09:40 <@jnewbery> I think there may be circumstances where you'd have gaps in your change keys, but it's less likely than with giving out addresses (where someone might just not pay you)
09:40 < amiti> although I guess you could form txns at different fee rates that get mined at different times and are thus seen on the chain out of order?
09:41 <@jnewbery> yes, true
09:41 -!- jkczyz_ [~jkczyz@135.84.132.56] has left #bitcoin-core-pr-reviews []     
09:41 < amiti> yeah I was wondering that... what happens in that case?
09:41 <@jnewbery> and if you RBF bump, then you use a new key for change I think
09:41 < jonatack> MarcoFalke: in commit faf7cc8 updating wallet_import_rescan.py::134, I was wondering, why is "rescan" changed to "expect_rescan" in ImportNode?
09:42 <@jnewbery> the logic in the core wallet is the same for both. We have a gap limit of 1000. Perhaps it could be reduced for internal keys
09:42 < jonatack> MarcoFalke: ImportNode = collections.namedtuple("ImportNode", "prune expect_rescan blockfilter")
09:42 <@jnewbery> has anyone spotted the bug in this PR yet? :)
09:42 <@jnewbery> (I've been giving you hints by talking about the keypool)
09:44 <@jnewbery> hint: why is important that TopUp() is called in the rescan loop?
09:44 -!- purposely [~purposely@141.255.166.195] has joined #bitcoin-core-pr-reviews     
09:45 < amiti> TopUpKeyPool()? or is there a different function TopUp() that I'm missing?
09:46 < jkczyz> https://github.com/bitcoin/bitcoin/blob/976cc766c42889b6d1042e4cda39045d5001f406/src/wallet/scriptpubkeyman.cpp#L1033
09:46 < amiti> ah thanks
09:46 <@jnewbery> https://github.com/bitcoin/bitcoin/blob/976cc766c42889b6d1042e4cda39045d5001f406/src/wallet/scriptpubkeyman.cpp#L293
09:46 <@jnewbery> that's where it's called from AddToWalletIfInvolvingMe()
09:47 <@jnewbery> what happens if you create a wallet, make a backup, then receive 1001 payments, and then try to restore from that backup?
09:48 < molz> one of them isn't in the backup
09:48 < pinheadmz> Are we not topping up every time a key is found in a block during rescan?
09:48 <@jnewbery> molz: none of the transactions are in the backup. The initial 1000 keys are in the backup
09:48 <@jnewbery> pinheadmz: you're getting close
09:48 < molz> so currently core wallet has deterministic backup so if we make a backup at the start of the wallet, it will automatically backup all our keys by the TopUp call?
09:49 <@jnewbery> molz: yes, the keys are deterministic, so when topping up the backup, you'll get the same new keypool keys
09:49 < molz> ah     
09:49 <@jnewbery> pinheadmz: in the new PR15845 logic, what are we matching the blocks against?
09:50 < pinheadmz> GetAllRelevantScriptPubKeys()
09:50 <@jnewbery> which returns a GCS filter
09:50 <@jnewbery> and where is that filter built?
09:50 < jkczyz> So the filter needs to be updated?
09:50 <@jnewbery> jkczyq: \o/
09:50 < pinheadmz> \o/ haha
09:51 < pinheadmz> I should've guessed that!
09:51 <@jnewbery> GetAllRelevantScriptPubKeys() is only called _once_, outside the rescan loop
09:51 <@jnewbery> that means as we advance through the blockchain, then it doesn't get updated
09:51 < pinheadmz> In bcoin we use internal bloom filters this same way, and in some cases the filter is not updated fast enough between blocks
09:52 < pinheadmz> But the filter comes from each block. So it's not the filter that needs to be updated it's the element list of keys to match against right?
09:53 < jkczyz> GCSFilter::ElementSet
09:53 <@jnewbery> pinheadmz: i'm not entirely sure what the right terminology is. The GCS is what the wallet creates to match against the block filter
09:53 < amiti> ah looks like ryanofsky changed it from TopUpKeyPool() to TopUp() yesterday, and I hadn't pulled the recent changed https://github.com/bitcoin/bitcoin/pull/17381/commits/491a599b37f3e3a648e52aebed677ca11b0615e2
09:54 <@jnewbery> From this comment, I think that an earlier version of the PR might have recalculated the GCS for each block: https://github.com/bitcoin/bitcoin/pull/15845#issuecomment-491539541
09:55 < pinheadmz> Hm but conceptually, the filter is derived once the block is verified and the filter for each block doesn't get changed or updated ?
09:55 <@jnewbery> which fixes this bug but is slow
09:55 <@jnewbery> pinheadmz: correct. The block filter doesn't ever change
09:56 <@jnewbery> 5 minutes left. Did anyont take a look at false-positive rates?
09:58 < pinheadmz> 😬
09:58 < jonatack> Any links on this apart from jimpo's comment? https://github.com/bitcoin/bitcoin/pull/15845#pullrequestreview-228848610
09:58 < ajonas> supposed to be 1/M right? and M is a param here: https://github.com/bitcoin/bitcoin/pull/12254/files#diff-e18b8a63a6ebd2d9ceedf323fd3a4861R77
09:58 <@jnewbery> ajonas: yes
09:58 < michaelfolkson> What's the latest update of BIP 157/158 in Core? I found this useful resource from some random called <@jnewbery> on StackExchange :) https://bitcoin.stackexchange.com/questions/86231/whats-the-distinction-between-bip-157-and-bip-158-are-they-supported-by-bitcoi
09:59 < michaelfolkson> There's still contention on serving filters right?
09:59 <@jnewbery> M is specified here: https://github.com/bitcoin/bips/blob/master/bip-0158.mediawiki#block-filters
09:59 < provoostenator> jnewbery: nice catch!
09:59 <@jnewbery> so a transaction that isn't in a block will return positive with probability 1/784931
10:01 <@jnewbery> I think that means that a full keypool of addresses will return positive with a probability of (1 - (784930/784931)^6000)
10:01 <@jnewbery> which I think is ~ 6000/784931
10:02 < ajonas> for those smarter than me Sipa has a write up on the optimization: https://gist.github.com/sipa/576d5f09c3b86c3b1b75598d799fc845
10:02 < jonatack> Interesting catch! We're missing tests that aren't seeing this bug, it would seem.
10:02 <@jnewbery> jonatack: it'd require a wallet with lots of transactions
10:02 <@jnewbery> the current import-rescan test doesn't have enough transactions to hit keypool topup issues
10:02 -!- diogosergio [~diogoserg@212.36.34.126] has quit [Ping timeout: 240 seconds]     
10:03 < jonatack> Chris Stewart, who has an open PR to add many property-based tests, was suggesting that BIP157/158 might be a fruitful area for rapidcheck testing.
10:03 <@jnewbery> We're over time, but I can go for a few more minutes if people have more questions
10:03 < pinheadmz> jnewbery: thanks again as always!!!
10:04 < jonatack> jnewbery: how did you catch this... code review?
10:05 <@jnewbery> jonatack: I implemented this PR a couple of years ago: https://github.com/bitcoin/bitcoin/pull/11022 so I know what sharp corners to look out for :)
10:06 < ysangkok> thanks!  awesome when there is a known bug that people have to find! so dramatic
10:06 < pinheadmz> Whoa was that with bip158 filters? A couple of years ago?
10:06 <@jnewbery> pinheadmz: no, just regular good old fashioned rescans
10:06 < pinheadmz> Ah     
10:07 <@jnewbery> ok, let's call time there
10:07 <@jnewbery> Thanks everyone. Fun meeting today!
10:07 <@jnewbery> #endmeeting
10:07 < jonatack> Nice :) and it was not caught yet in the review until now. Really good session.
10:07 < michaelfolkson> Thanks, fascinating
10:07 < molz> so the bug is not related to the filters?
10:07 < amiti> thanks !
10:07 < jonatack> Thanks!
10:08 <@jnewbery> I spotted it when I reviewed it last week but didn't want to spoil the fun for you all :)
10:08 < provoostenator> jonatack jnewbery keypool size on regtest is tiny (10?), so should be easy to test this
10:08 < jonatack> jnewbery: nice one
10:08 < jonatack> provoostenator: oh interesting
10:08 -!- pinheadmz [~pinheadmz@172.98.82.141] has quit [Quit: Mutter: www.mutterirc.com]     
10:09 < molz> would it be better if the keypool is smaller ? like 500 keys?
10:09 <@jnewbery> I'll try to get notes and questions up before the weekend. Please ping me if you want to host. I think lots of you are ready.
10:09 <@jnewbery> I'm happy to help you prepare and help with notes and questions
10:10 <@jnewbery> molz: I think 1000 is too large for most people. gmax disagrees: https://github.com/bitcoin/bitcoin/pull/15845#issuecomment-485160938
10:11 -!- michaelfolkson [~textual@62.60.63.228] has quit [Quit: Sleep mode]     
10:11 < molz> jnewbery, yea i was around the bitcoin channels when this was changed, i think because many business owners lost funds thinking if they backup once (100 keypool then) then the rest of their money was backed up
10:11 <@jnewbery> oh. One more thing
10:11 <@jnewbery> It turns out 17:00 UTC isn't ideal for me now that daylight savings is over
10:11 < jonatack> Same
10:12 < sebastianvstaa> same :)
10:12 <@jnewbery> would changing it to 18:00 preclude anyone from joining us?
10:12 < purposely> Not an issue for you,  Maybe a poll would help
10:13 < purposely> not an issue for me*
10:13 < provoostenator> molz: a potential followup to make it snappier, is to have two scan passes. One with very small lookahead window, and then one with a bigger window just in case.
10:13 < molz> provoostenator, how do you do this in core?
10:13 <@jnewbery> let me know if 18:00 doesn't work for you. If I don't hear from anyone, I'll update the website to say 18:00 this weekend
10:14 < provoostenator> I even thought about adding a wallet based cache to the REST address lookup API, which could do a just-in-time rescan. Then you wouldn't need electrum server :-)
10:14 < jonatack> jnewbery: yes, i'd say change it, communicate it well, and see if anyone complains
10:14 < provoostenator> (reckless idea, but still)
10:14 < purposely> if time changes, let's by loud about changing the time
10:14 <@jnewbery> I'm not going to do a poll. Just let me know if 18:00 doesn't work
10:16 < molz> jnewbery, thanks for doing this for us, I've been lurking until today, you've been doing a wonderful job, i really enjoy reading the convo and it helps me learn :D
10:17 < lightlike> 18:00 is much better for me as well.
10:17 <@jnewbery> molz: thanks. Glad you're taking a more active part now!
10:17 <@jnewbery> I enjoy doing these. I think it's a really good way for us all to learn from each other
10:19 <@jnewbery> I would really like it if I could convince more of you to host. I think having to host makes you really understand the PR and it's a great way to learn.
10:19 <@jnewbery> (but I understand most of you are doing this in your free time, and it's a big ask)
10:19 < jonatack> +1 for a great way to learn
10:19 < MarcoFalke> +1     
10:19 < MarcoFalke> This one was eye-opening
10:19 < sebastianvstaa> +1     
10:30 -!- michaelfolkson [~textual@85.255.235.91] has joined #bitcoin-core-pr-reviews     
10:30 -!- sebastianvstaa [~sebastian@HSI-KBW-091-089-090-072.hsi2.kabelbw.de] has quit [Quit: Leaving]     
10:33 < ysangkok> what is the URL for the channel logs?
10:35 < jonatack> ysangkok: this channel isn't logged, but the meeting log will be added to the website
10:35 < jonatack> ysangkok: ah, it's up: https://bitcoincore.reviews/15845.html
10:39 -!- pinheadmz [~matthewzi@239-196.icannmeeting.org] has joined #bitcoin-core-pr-reviews     
10:44 -!- avril [wha@unaffiliated/avril] has joined #bitcoin-core-pr-reviews     
11:21 -!- b10c [~Thunderbi@i577BC684.versanet.de] has quit [Ping timeout: 240 seconds]     
11:22 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews     
11:28 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has quit []     
11:28 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews     
11:32 -!- purposely [~purposely@141.255.166.195] has quit [Quit: purposely]     
11:43 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has quit []     
11:44 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews     
11:50 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has quit []     
11:51 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews     
11:54 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has quit [Remote host closed the connection]     
11:59 -!- zenogais [~user@cpe-76-175-74-114.socal.res.rr.com] has left #bitcoin-core-pr-reviews ["ERC (IRC client for Emacs 26.3)"]     
12:39 -!- Talkless [~Talkless@hst-227-49.splius.lt] has quit [Quit: Konversation terminated!]     
12:43 -!- diogosergio [~diogoserg@176.24.23.243] has joined #bitcoin-core-pr-reviews     
12:50 -!- diogosergio [~diogoserg@176.24.23.243] has quit [Ping timeout: 268 seconds]     
12:58 -!- michaelfolkson [~textual@85.255.235.91] has quit [Quit: Sleep mode]     
12:58 -!- diogosergio [~diogoserg@176.24.23.243] has joined #bitcoin-core-pr-reviews     
13:33 -!- diogosergio [~diogoserg@176.24.23.243] has quit [Ping timeout: 240 seconds]     
13:48 -!- davterra [~none@209.58.188.204] has joined #bitcoin-core-pr-reviews     
13:51 < molz> hm .. wondering if i'm doing something wrong, i don't see this syncing faster than a node without this PR
13:53 < molz> i'm running the node on mainnet with -listen=0 -blockfilterindex=1
13:54 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews     
14:00 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has quit [Ping timeout: 264 seconds]     
14:05 -!- diogosergio [~diogoserg@176.24.23.243] has joined #bitcoin-core-pr-reviews     
14:24 -!- pinheadmz [~matthewzi@239-196.icannmeeting.org] has quit [Quit: pinheadmz]     
14:29 -!- slivera [slivera@gateway/vpn/privateinternetaccess/slivera] has joined #bitcoin-core-pr-reviews     
15:19 -!- soju [~soju@2601:640:8780:6d90:eca0:7eb0:4e45:8ac7] has joined #bitcoin-core-pr-reviews     
15:23 -!- openoms [~quassel@cpc115066-stok20-2-0-cust313.1-4.cable.virginm.net] has joined #bitcoin-core-pr-reviews     
15:53 -!- michaelfolkson [~textual@85.255.235.32] has joined #bitcoin-core-pr-reviews     
15:53 -!- diogosergio [~diogoserg@176.24.23.243] has quit [Ping timeout: 240 seconds]     
15:56 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews     
15:56 -!- michaelfolkson [~textual@85.255.235.32] has quit [Client Quit]     
15:57 -!- diogosergio [~diogoserg@176.24.23.243] has joined #bitcoin-core-pr-reviews     
16:01 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has quit [Ping timeout: 276 seconds]     
16:31 -!- diogosergio [~diogoserg@176.24.23.243] has quit [Ping timeout: 276 seconds]     
16:45 -!- soju [~soju@2601:640:8780:6d90:eca0:7eb0:4e45:8ac7] has quit [Remote host closed the connection]     
16:50 -!- michaelfolkson [~textual@85.255.235.32] has joined #bitcoin-core-pr-reviews     
16:51 -!- diogosergio [~diogoserg@176.24.23.243] has joined #bitcoin-core-pr-reviews     
17:00 -!- diogosergio [~diogoserg@176.24.23.243] has quit [Ping timeout: 240 seconds]     
17:05 -!- pinheadmz [~matthewzi@5.181.233.92] has joined #bitcoin-core-pr-reviews     
18:07 -!- openoms [~quassel@cpc115066-stok20-2-0-cust313.1-4.cable.virginm.net] has quit [Ping timeout: 265 seconds]     
18:08 -!- openoms [~quassel@cpc115066-stok20-2-0-cust313.1-4.cable.virginm.net] has joined #bitcoin-core-pr-reviews     
18:25 -!- michaelfolkson [~textual@85.255.235.32] has quit [Quit: Sleep mode]     
18:29 -!- lightlike [~lightlike@2001:16b8:577c:d900:11d3:b4a1:4c35:e2b8] has quit [Quit: Leaving]     
19:01 -!- openoms [~quassel@cpc115066-stok20-2-0-cust313.1-4.cable.virginm.net] has quit [Quit: No Ping reply in 180 seconds.]     
19:01 -!- openoms [~quassel@cpc115066-stok20-2-0-cust313.1-4.cable.virginm.net] has joined #bitcoin-core-pr-reviews     
19:05 -!- emilengler_ [~emilengle@unaffiliated/emilengler] has joined #bitcoin-core-pr-reviews     
19:08 -!- emilengler [~emilengle@unaffiliated/emilengler] has quit [Ping timeout: 265 seconds]     
19:13 -!- felixfoertsch23 [~felixfoer@2001:16b8:501a:8100:651b:bc65:1d2d:137b] has joined #bitcoin-core-pr-reviews     
19:13 -!- felixfoertsch [~felixfoer@2001:16b8:506f:100:651b:bc65:1d2d:137b] has quit [Ping timeout: 264 seconds]     
19:20 -!- slivera [slivera@gateway/vpn/privateinternetaccess/slivera] has quit [Quit: Leaving]     
21:03 -!- soju__ [~soju@2601:640:8780:6d90:8d5f:7c5a:ed4:a0bf] has joined #bitcoin-core-pr-reviews     
21:07 -!- soju__ [~soju@2601:640:8780:6d90:8d5f:7c5a:ed4:a0bf] has quit [Ping timeout: 245 seconds]     
21:22 -!- felixfoertsch23 [~felixfoer@2001:16b8:501a:8100:651b:bc65:1d2d:137b] has quit [Quit: ZNC 1.7.3 - https://znc.in]     
21:22 -!- felixfoertsch [~felixfoer@2001:16b8:501a:8100:91d:b12b:f153:ede7] has joined #bitcoin-core-pr-reviews     
21:25 -!- openoms [~quassel@cpc115066-stok20-2-0-cust313.1-4.cable.virginm.net] has quit [Quit: No Ping reply in 180 seconds.]     
21:26 -!- openoms [~quassel@cpc115066-stok20-2-0-cust313.1-4.cable.virginm.net] has joined #bitcoin-core-pr-reviews     
21:58 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews     
22:03 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has quit [Ping timeout: 276 seconds]     
22:29 -!- davterra [~none@209.58.188.204] has quit [Remote host closed the connection]     
23:04 -!- soju__ [~soju@2601:640:8780:6d90:8d5f:7c5a:ed4:a0bf] has joined #bitcoin-core-pr-reviews     
23:56 -!- soju__ [~soju@2601:640:8780:6d90:8d5f:7c5a:ed4:a0bf] has quit [Remote host closed the connection]     
23:59 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews