< bitcoin-git> [bitcoin] achow101 opened pull request #17886: Restore English translation option (master...fix-en-translate) https://github.com/bitcoin/bitcoin/pull/17886
< promag> ariard could be member in gh
< bitcoin-git> [bitcoin] kallewoof opened pull request #17887: bug-fix macos: give free bytes to F_PREALLOCATE (master...macos-f_preallocate_fix) https://github.com/bitcoin/bitcoin/pull/17887
< bitcoin-git> [bitcoin] xyephy opened pull request #17888: Merge (master...master) https://github.com/bitcoin/bitcoin/pull/17888
< bitcoin-git> [bitcoin] fanquake closed pull request #17888: Merge (master...master) https://github.com/bitcoin/bitcoin/pull/17888
< bitcoin-git> [bitcoin] promag opened pull request #17889: wallet: Improve CWallet:MarkDestinationsDirty (master...2020-01-wallettx-iscacheempty) https://github.com/bitcoin/bitcoin/pull/17889
< kallewoof> Why would bitcoin core reopen old rev files after closing their blk files? #17890
< gribble> https://github.com/bitcoin/bitcoin/issues/17890 | Non-latest rev files are sometimes re-opened but never re-flushed . Issue #17890 . bitcoin/bitcoin . GitHub
< sipa> kallewoof: rev files are written at activation time, which is not the same time as the blocks are written
< sipa> (not sure that explains what you're seeing, though0
< kallewoof> sipa: we need to flush them afterwards then. maybe always flush after each write, if it is not the current one
< kallewoof> sipa: i am seeing 2 separate bugs. 1 bug is that we over-allocate by (file size) amount, and 2nd is that we are not flushing afterwards, leaving some of the rev files in the overblown-size state.
< sipa> hmm, we don't mark block index entries dirty when the corresponding block is activated/deactivated?
< kallewoof> sipa: we only ever flush a undo file when we close its block file.
< kallewoof> sipa: "Leaving block X" is when we flush both blk and rev. Never again after that
< sipa> ah, that seems wrong
< sipa> i'll have a look
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #17891: scripted-diff: Replace CCriticalSection with RecursiveMutex (master...2001-c2-CC) https://github.com/bitcoin/bitcoin/pull/17891
< kallewoof> sipa: ok. I'm looking too
< kallewoof> sipa: I think part of the issue is that blocks are stored to disk and there is a lag between that and the block being connected (probably due to sig checks / utxo etc), and since undo data is written then (at activation time as you said), the system will move on to the next block file but the activation will still be working against the previous one.
< kallewoof> I saw the first encounter of 'write to an older rev file' right after closing block file 0, which said it had 'heights=0...119975', but right before that line there was an UpdateTip new best for height 119963, and my code broke when it was about to find undo pos for height 119965.
< kallewoof> s/broke/hit my breakpoint/g
< kallewoof> looks like a decent fix here is to flush the blk/undo when the activation occurs (as well, probably, to cover both cases).
< kallewoof> i mean, when the activation goes from 'X' to 'X+1' in terms of the block file #.
< sipa> kallewoof: iirc we only have one set of "dirty" files which are synced at flush time, which covers both blk and rev files
< sipa> it seems we'll need to split that up
< kallewoof> sipa: there is a setDirtyBlockIndex which is then chucked at a txdb.cpp method which writes a bunch. It's not immediately clear to me if those are the rev/blk or the index stuff.
< kallewoof> the only place we flush (truncate to actual used size) the blk/rev is in that one place in validation.cpp. there is another place which calls but with finalize=false which doesn't do that.
< sipa> kallewoof: ah right
< kallewoof> Contrary to what the block file tostring() would have you believe, it seems that blocks are not actually in a strict range inside the blk files. I see block 119776 in blk1 and 119777 in blk0.
< goatpig_> hasnt been the case since header first
< kallewoof> i have a sorta working patch but it needs to be reworked around the fact blocks are jumbled. see https://github.com/kallewoof/bitcoin/tree/200108-reflush-undo
< kallewoof> I think the straightforward solution is to determine the last block in the old rev file beforehand and to have the write for that trigger the flush. But it's 3 am now. /waves
< bitcoin-git> [bitcoin] kallewoof opened pull request #17892: bug-fix: re-flush undo files after they have been re-opened (master...200108-reflush-undo) https://github.com/bitcoin/bitcoin/pull/17892
< bitcoin-git> [bitcoin] IPGlider closed pull request #17884: utils: Use fallback for macOS in AllocateFileRange (master...workaround_apfs_allocation_problem) https://github.com/bitcoin/bitcoin/pull/17884
< sipa> e er5~
< sipa> oops
< jeremyrubin> Does anyone have any trouble with univalue encoding VARR types?
< jeremyrubin> I'm working on an RPC and I'm getting: vector::_M_range_check: __n (which is 1) >= this->size() (which is 1)
< jeremyrubin> (sidebar: the NONFATAL stuff makes debugging these sorts of problems harder, we should add flags to re-enable panic)
< sipa> it's probably good to make NONFATAL failures turn into assertion failures in debug builds
< bitcoin-git> [bitcoin] meshcollider pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/e8e79958a7b2...bcb4cdcca321
< bitcoin-git> bitcoin/master 0950245 Gregory Sanders: IsUsedDestination should count any known single-key address
< bitcoin-git> bitcoin/master bcb4cdc Samuel Dobson: Merge #17621: IsUsedDestination should count any known single-key address
< bitcoin-git> [bitcoin] meshcollider merged pull request #17621: IsUsedDestination should count any known single-key address (master...actually_no_reuse) https://github.com/bitcoin/bitcoin/pull/17621
< jeremyrubin> sipa: what's the debug flag named (not NDEBUG)
< sipa> jeremyrubin: --enable-debug sets some flags; it could set another one that specifically control this behaviour
< bitcoin-git> [bitcoin] meshcollider pushed 4 commits to master: https://github.com/bitcoin/bitcoin/compare/bcb4cdcca321...45f151913ef5
< bitcoin-git> bitcoin/master 75a5e47 Gregory Sanders: Change bumpfee to use watch-only funds for legacy watchonly wallets
< bitcoin-git> bitcoin/master e9b4f94 Gregory Sanders: bumpfee: Return PSBT when wallet has privkeys disabled
< bitcoin-git> bitcoin/master 091a876 Gregory Sanders: Test watchonly wallet bumpfee with PSBT return
< bitcoin-git> [bitcoin] meshcollider merged pull request #16373: bumpfee: Return PSBT when wallet has privkeys disabled (master...bump_psbt) https://github.com/bitcoin/bitcoin/pull/16373
< jeremyrubin> Ah; I had a variable x and a string "x" and I did x.pushKV("label", x)
< bitcoin-git> [bitcoin] meshcollider pushed 5 commits to master: https://github.com/bitcoin/bitcoin/compare/45f151913ef5...7ea3b85ecf7b
< bitcoin-git> bitcoin/master 7851f14 Jon Atack: rpc: incorporate review feedback from PR 17283
< bitcoin-git> bitcoin/master 60aba1f Jon Atack: rpc: simplify getaddressinfo labels, deprecate previous behavior
< bitcoin-git> bitcoin/master 8bb405b Jon Atack: test: getaddressinfo labels purpose deprecation test
< bitcoin-git> [bitcoin] meshcollider merged pull request #17578: rpc: simplify getaddressinfo labels, deprecate previous behavior (master...deprecate-getaddress-info-labels-purpose) https://github.com/bitcoin/bitcoin/pull/17578
< bitcoin-git> [bitcoin] meshcollider pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/7ea3b85ecf7b...cab3859a356d
< bitcoin-git> bitcoin/master e1e1442 Gregory Sanders: Activate no-privkey -> ISMINE_WATCH_ONLY behavior for LegacySPKM only
< bitcoin-git> bitcoin/master cab3859 Samuel Dobson: Merge #17677: Activate watchonly wallet behavior for LegacySPKM only
< bitcoin-git> [bitcoin] meshcollider merged pull request #17677: Activate watchonly wallet behavior for LegacySPKM only (master...legacy_spkm_watchonly) https://github.com/bitcoin/bitcoin/pull/17677
< hebasto> ryanofsky: hmm, why test/functional/feature_config_args.py passed on 67518f7cc61bf59ddfa0fd7c8dbbdec3653b9556 ?
< hebasto> I see now: test is broken.
< ryanofsky> test should be unaffected by that change, do you see a break somewhere?
< hebasto> yes: "-connect=0x7fff50369968" != "-connect=0"
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/cab3859a356d...fcef6dbc15ef
< bitcoin-git> bitcoin/master 77ef48d Jon Atack: gitignore: ignore fuzz binaries, remove test_bitcoin_fuzzy
< bitcoin-git> bitcoin/master fcef6db fanquake: Merge #17452: test: update fuzz directory in .gitignore
< bitcoin-git> [bitcoin] fanquake merged pull request #17452: test: update fuzz directory in .gitignore (master...update-fuzz-gitignore) https://github.com/bitcoin/bitcoin/pull/17452
< ryanofsky> Oh I see how that would happen, it should not be a problem in the current PR.
< hebasto> going to submit a pr to fix test
< ryanofsky> in the commit you mentioned, value is a pointer to a string, and it was printing the pointer address instead of the string on: LogPrintf("Warning: parsed potentially confusing double-negative -%s=%s\n", key, value);
< hebasto> correct
< ryanofsky> oh I see, test could be fixed to more robust and not accept trailing characters after 0
< captjakk> does bitcoind not support the newer 54 char onion addresses for tor?
< sipa> no
< sipa> needs bip 155 in order to represent those
< captjakk> what's the timeline on 155, and is it something that could benefit from engineering contributions right now, or is it still being debated what the best approach is to get it in?
< sipa> i don't know if anyone is working on it right now
< sipa> i don't think there are any known problems with it
< captjakk> kk, so if I submit a patch implementing it, are you optimistic it would be merged assuming no implementation problems?
< captjakk> I'd imagine since this isn't a consensus issue, the vuln surface is substantially smaller