--- 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