< bitcoin-git> [bitcoin] 251Labs opened pull request #12436: [rpc] Adds a functional test to validate the transaction version number in the RPC output (master...issue/11561/test-negative-transaction-version-numbers) https://github.com/bitcoin/bitcoin/pull/12436
< bitcoin-git> [bitcoin] jeffrade opened pull request #12437: [Trivial] Simplify if-else blocks and more descriptive variable naming (master...test_runner_refactor) https://github.com/bitcoin/bitcoin/pull/12437
< bitcoin-git> [bitcoin] MeshCollider opened pull request #12438: [Tests] Fix trivial typo in test_runner.py causing error (master...201802_fix_testrunner_typo) https://github.com/bitcoin/bitcoin/pull/12438
< esotericnonsense> are the tx in getblocktemplate ordered according to the highest feerate of the packages?
< gmaxwell> packages are ordered by package feerate.
< gmaxwell> it causes a triangle wave shape in the transaction feerate in blocks.
< esotericnonsense> yeah. that's what I thought (just guessing based on eyeballing the pattern without looking at txids)
< gmaxwell> because coinbase txn size is not known when a block is being constructed we've generally tried to preserve the property that a truncation of a block is about as good of a solution as if you targeted a smaller block.
< gmaxwell> I think probably in the future we should give up on that property, and instead have mining protocols where the size of the coinbase txn will be known exactly.
< esotericnonsense> why is the coinbase size not known? can't think of anything off the top of my head that would make its' size vary
< esotericnonsense> aside from adding extra outputs etc, but then it's still known, you'd just need to tell gbt
< gmaxwell> coinbase transaction, none of it is known to gbt... how many outputs will it make? what scriptpubkey styles will they be?
< gmaxwell> how much extra-nonce space will you have? none of that is provided to GBT.
< esotericnonsense> right yeah that makes sense. I misinterpreted what you were saying. coinbase size isn't known _by gbt_ but it can be known by the miner.
< grafcaps> gbt = get block template?
< esotericnonsense> is anyone out there charting gbt feerates live? would it be useful to anyone?
< esotericnonsense> (i've got a flask table thing working but making it into a chart probably wouldn't be much effort)
< gmaxwell> esotericnonsense: right, it's known by the miner.
< gmaxwell> it's just an interface design thing. in a lot of ways GBT reflects how eligius used to work.
< gmaxwell> coinbases were produced by their payout algorihim and changed very rapidly in realtime.
< gmaxwell> It _could_ have worked so that the size was known to GBT and just the specific outputs and amounts changed... but there wasn't any obvious gain in doing that back then... blocks were not full due to hardcoded underuse, fees were negligible.
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/e782099a151a...b2089c51cc4a
< bitcoin-git> bitcoin/master ada1af6 MeshCollider: Fix typo in test_runner.py causing error
< bitcoin-git> bitcoin/master b2089c5 MarcoFalke: Merge #12438: [Tests] Fix trivial typo in test_runner.py causing error...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #12438: [Tests] Fix trivial typo in test_runner.py causing error (master...201802_fix_testrunner_typo) https://github.com/bitcoin/bitcoin/pull/12438
< esotericnonsense> heh, watching this is crazy. i've never really looked before. there's a tx here with 2300 satoshi per byte ? wtf
< esotericnonsense> i suppose that's "only" $60 or so.
< esotericnonsense> could be improved by log scale but whatever. https://esotericnonsense.com/dev/ url will probably change etc.
< meshcollider> esotericnonsense: that looks cool
< booyah> fyi, enemies of bitcoin at r/btc claim also that "you" kept commiting changes to "trunk", prohibiting poor Gavin from getting his changes in (cos he needed to constantly rebase?). what the ;)
< Randolf> Oh, isn't r/btc that Reddit community that Roger Ver runs from a distance?
< booyah> Randolf: yes
< Randolf> When it comes to "enemies of Bitcoin," it seems that the only name that ever comes up is Roger Ver.
< booyah> Randolf: yes (also he is openly it's owner). That's why it's just FYI, no one pays attention to it. Just thought it's funny, idea of "buuu huuu evil developers keep develoing stuff and I need to rebase my PR". ( https://www.reddit.com/r/btc/comments/7xjy03/did_you_know_that_before_the_scaling_debate_got/du9g889/ )
< luke-jr> esotericnonsense: there is no guarantee on order of transactions in GBT, other than that they are valid
< sipa> Randolf, booyah: please stick to technical topics here
< * Randolf> nods agreeably
< wumpus> booyah: lol! people kept contributing to the project, so the poor guy couldn't get his changes in :(
< wumpus> booyah: one thing seems to be consistent, they never really get open source development
< wumpus> or what Gavin's task was in the first place. for that matter
< wumpus> cfields: our level of misunderanding of boost::interprocess::file_lock is phenomenal, can't we just delete that function :/
< wumpus> cfields: I think I'll just revert #12422 to the OpenBSD compile fix that it was, for 0.16, and leave the rest for master/0.17
< gribble> https://github.com/bitcoin/bitcoin/issues/12422 | util: Make LockDirectory thread-safe, consistent, and fix OpenBSD 6.2 build by laanwj · Pull Request #12422 · bitcoin/bitcoin · GitHub
< wumpus> cfields: the 'other process' test likely should have another case, that after process A giving up the locks (e.g. if it was only probing), B should succeed in getting it
< fanquake> wumpus I have a fix for the miniupnpc compile issue, will push up shortly
< wumpus> fanquake: great!
< fanquake> wumpus 12417 Should be able to go in now
< provoostenator> Rebasing is a pain, but my impression so far is that most of the time the stuff that's ready first goes in first.
< fanquake> provoostenator Are you just talking generally, or in regards to specific PR?
< provoostenator> I mean in general.
< meshcollider> yeah PRs which need rebasing or are failing travis tests tend to get fewer reviews than those which are ready from what I've seen
< provoostenator> That was ambiguous. Rebasing is a pain for some types of PR's and if Travis is having a bad day, but often a simple chore. In general when a PR has had enough review and feedback is addressed quickly enough, it goes in quickly, and then other PR's need to rebase.
< meshcollider> Oh I see what you meant
< provoostenator> 4 open PR's on 0.16.0. I'll try to test #12427
< gribble> https://github.com/bitcoin/bitcoin/issues/12427 | Make signrawtransaction accept P2SH-P2WSH redeemscripts by sipa · Pull Request #12427 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/b2089c51cc4a...ae0fbf09817f
< bitcoin-git> bitcoin/master deee216 Douglas Roark: Delete mac_alias patch...
< bitcoin-git> bitcoin/master fc1bfcf Douglas Roark: Update mac_alias to 2.0.7
< bitcoin-git> bitcoin/master ae0fbf0 Wladimir J. van der Laan: Merge #12417: Upgrade mac_alias to 2.0.7...
< bitcoin-git> [bitcoin] laanwj closed pull request #12417: Upgrade mac_alias to 2.0.7 (master...master_del_mac_alias) https://github.com/bitcoin/bitcoin/pull/12417
< wumpus> cfields: so I think you're right on the longer run - then we likely want the wallets (and init, for the main data dir) themselves to carry a RAII lock on the data directory instead of the funky map
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/ae0fbf09817f...737ed8bb77d1
< bitcoin-git> bitcoin/master 2e9406c João Barbosa: Interrupt loading thread after shutdown request
< bitcoin-git> bitcoin/master 737ed8b Wladimir J. van der Laan: Merge #12415: Interrupt loading thread after shutdown request...
< bitcoin-git> [bitcoin] laanwj closed pull request #12415: Interrupt loading thread after shutdown request (master...2018-02-shutdown) https://github.com/bitcoin/bitcoin/pull/12415
< bitcoin-git> [bitcoin] laanwj pushed 1 new commit to 0.16: https://github.com/bitcoin/bitcoin/commit/f8938248ef37f67986e36ef897a548123b2d7a36
< bitcoin-git> bitcoin/0.16 f893824 João Barbosa: Interrupt loading thread after shutdown request...
< wumpus> re: #12349, can we be sure this issue is really solved after that?
< gribble> https://github.com/bitcoin/bitcoin/issues/12349 | shutdown: fix crash on shutdown with reindex-chainstate by theuni · Pull Request #12349 · bitcoin/bitcoin · GitHub
< wumpus> if not, I think we should think about it somewhat longer how to effectively fix it, and remove the 0.16.0 tag, we shouldn't merge fix after fix to different parts of the code without a clear idea if it's really fixed
< wumpus> let's discuss it at the meeting
< promag> wumpus: will look #12349
< gribble> https://github.com/bitcoin/bitcoin/issues/12349 | shutdown: fix crash on shutdown with reindex-chainstate by theuni · Pull Request #12349 · bitcoin/bitcoin · GitHub
< promag> what is the idea to fix "ERROR: src/crypto/ctaes is not a subtree"? full clone?
< wumpus> promag: I really hate that solution, but yes, that'd solve it
< promag> is there an alternative?
< promag> git fetch commit_id && git reset --hard FETCH_HEAD ?
< wumpus> see #12388
< gribble> https://github.com/bitcoin/bitcoin/issues/12388 | travis failure on rc3: "src/crypto/ctaes is not a subtree" · Issue #12388 · bitcoin/bitcoin · GitHub
< wumpus> I think the scalable solution would be to skip the subtree check when the subtree is buried deeper than the shallow checkout
< wumpus> after all, the check is there to catch PRs that make changes to subtrees in the wrong way
< wumpus> if a change is buried below many other PRs then we can already be sure there's nothing to check
< wumpus> otherwise they wouldn'th ave been merged...
< bitcoin-git> [bitcoin] laanwj pushed 1 new commit to 0.16: https://github.com/bitcoin/bitcoin/commit/e2431d144aa84909b106a767634126ad5821dc90
< bitcoin-git> bitcoin/0.16 e2431d1 MarcoFalke: travis: Full clone for git subtree check...
< wumpus> but I'll cherry pick that for now to the 0.16 branch
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/737ed8bb77d1...bfa39114e2cf
< bitcoin-git> bitcoin/master fa27623 MarcoFalke: qt: Initialize members in WalletModel
< bitcoin-git> bitcoin/master bfa3911 Wladimir J. van der Laan: Merge #12426: qt: Initialize members in WalletModel...
< bitcoin-git> [bitcoin] laanwj closed pull request #12426: qt: Initialize members in WalletModel (master...Mf1802-qtInitializeMembersWalletModel) https://github.com/bitcoin/bitcoin/pull/12426
< bitcoin-git> [bitcoin] laanwj pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/bfa39114e2cf...fd65937ec601
< bitcoin-git> bitcoin/master c04e0f6 Ben Woosley: Fix 'mempool min fee not met' debug output...
< bitcoin-git> bitcoin/master 8b8a1c4 Ben Woosley: Add test for 'mempool min fee not met' rpc error
< bitcoin-git> bitcoin/master bb00c95 Ben Woosley: Consistently use FormatStateMessage in RPC error output...
< bitcoin-git> [bitcoin] laanwj closed pull request #12356: Fix 'mempool min fee not met' debug output (master...minfee-message) https://github.com/bitcoin/bitcoin/pull/12356
< bitcoin-git> [bitcoin] laanwj pushed 1 new commit to 0.16: https://github.com/bitcoin/bitcoin/commit/5e40e64face29169ef355856dc0db46a98061046
< bitcoin-git> bitcoin/0.16 5e40e64 Wladimir J. van der Laan: travis: Don't fetch --unshallow when no longer shallow-cloning...
< provoostenator> Just got another whitespace error back from Travis. Anyone have a chance to bless or find problems with #12098 and #12097 so I can trivially prevent that on MacOS?
< gribble> https://github.com/bitcoin/bitcoin/issues/12098 | [scripts] lint-whitespace: add param to check last N commits by Sjors · Pull Request #12098 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/12097 | [scripts] lint-whitespace: use perl instead of grep -P by Sjors · Pull Request #12097 · bitcoin/bitcoin · GitHub
< ProfMac> I think the code lets us generate a block with any prev-block-hash. That is, generate a fork at any block. In practice, this seems to occur only at the best tip when two block arrive nearby in time. Is it improper to think more restrictive thoughts that a new block can only point back to a tip?
< sipa> "the code" ?
< ProfMac> reference client, satoshi client ...
< sipa> in what context?
< sipa> there are many tests that mine forks
< ProfMac> Do those tests only grow existing forks, or do they create new forks somewhere on the backbone?
< sipa> backbone?
< ProfMac> Yeah, backbone. "best chain"
< sipa> oh
< sipa> arbitrary
< sipa> look at p2p-fullnode.py
< ProfMac> I haven't looked into the test software source yet. Thanks, I'll read that.
< sipa> but i don't understand your question at all
< sipa> forks happen all the time in the network
< sipa> and we need to deal with them
< sipa> even if only software existed that extended the tip, this would still be the case, simply due to the finite speed of communication
< sipa> (you may not have heard about the latest block someone created)
< ProfMac> Yes. And yes. I just wondered, in practice, do blocks show up that fork back 6 or 8 blocks from the best top.
< ProfMac> *tip
< sipa> yes
< ProfMac> meaning, create a new fork 6 or 8 blocks back. Not create a fork that ages to become that far back.
< sipa> very rarely though
< ProfMac> Yes, well, they would seem to be a bit pointless.
< sipa> let's move this to #bitcoin
< ProfMac> ok.
< promag> jonasschnelli: friendly ping #11882
< gribble> https://github.com/bitcoin/bitcoin/issues/11882 | Disable default fallbackfee on mainnet by jonasschnelli · Pull Request #11882 · bitcoin/bitcoin · GitHub
< promag> cfields: ignore my comment
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/fd65937ec601...d09968f4d006
< bitcoin-git> bitcoin/master 65682da Sjors Provoost: [tests] bind functional test nodes to
< bitcoin-git> bitcoin/master d09968f Wladimir J. van der Laan: Merge #12200: Bind functional test nodes to
< bitcoin-git> [bitcoin] laanwj closed pull request #12200: Bind functional test nodes to (master...test-framework-bind) https://github.com/bitcoin/bitcoin/pull/12200
< cfields> wumpus: here now. my sleep schedule is all out of whack :\
< sipa> morning!
< cfields> heh
< cfields> wumpus: yes, every time I think I understand it, something new pops up and totally confuses me again. Sorry for dragging you down with me :)
< cfields> wumpus: glad to see that we agree about longer-term. I was also thinking that we should move the wallet locks out of the map and into wallet as a follow-up.
< cfields> wumpus: re #12349, I'll test with current master now
< gribble> https://github.com/bitcoin/bitcoin/issues/12349 | shutdown: fix crash on shutdown with reindex-chainstate by theuni · Pull Request #12349 · bitcoin/bitcoin · GitHub
< wumpus> cfields: I feel the same about programming, and c++ in particular, there's always some new and unexpected situation that comes up
< wumpus> I also have the feeling boost doesn't help here :-)
< cfields> heh
< wumpus> might make sense to implement this using the posix and win32 API at some point so that we actually know for sure what it's doing
< wumpus> for now, having a test helps
< * wumpus> wonders what the windows equivalent of socketpair+fork is, e.g. create a process (doesn't have to copy the current one's state) with bidirectional pipe attached
< cfields> no clue. I assume something 100% winapi with no resemblence to posix :(
< wumpus> oh cool, we can do 90's style winapi with WM_* messages
< cfields> heh. Probably something like LockFile2Ex3Real()
< cfields> Where obviously LockFileEx might still work, but only up to Win3.1.
< cfields> it was dark days, dealing with that crap :)
< wumpus> yes :-)
< achow101> Meeting?
< Murch> aye
< wumpus> #startmeeting
< lightningbot> Meeting started Thu Feb 15 19:01:13 2018 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< wumpus> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator
< cfields> hi
< wumpus> we're almost ready to tag rc4, the still open PRs could still use some review https://github.com/bitcoin/bitcoin/pulls?q=is%3Aopen+is%3Apr+milestone%3A0.16.0
< promag> hi
< kanzure> hi.
< luke-jr> hi
< cfields> I'm not sure if I'm happy to see people testing the RCs, or scared of how many things keep cropping up :\
< BlueMatt> heh
< BlueMatt> yea
< wumpus> I'd like to discuss #12349
< gribble> https://github.com/bitcoin/bitcoin/issues/12349 | shutdown: fix crash on shutdown with reindex-chainstate by theuni · Pull Request #12349 · bitcoin/bitcoin · GitHub
< BlueMatt> i hate that fix, but it should be sufficient :(
< wumpus> #topic reindex-chainstate crash
< sipa> present
< wumpus> so we committed a fix for that in an earlier rc, but apparently the issue still exists
< BlueMatt> yes, the fix was for a highly-related bug
< BlueMatt> well, ok, essentially, either the same crash triggered by doing reindex, or the crash triggered by simply quickly quitting
< BlueMatt> so its already fixed for fast-quit
< wumpus> is it clear what the issue is in the first place? it seems it's causing workarounds to be spread over the code to 'fix' it
< wumpus> I'm not really happy to see that
< BlueMatt> yes, the issue is that we call Flush during shutdown before we've loaded genesis block
< cfields> wumpus: I just commented on the PR, I'll re-paste here for a little context:
< cfields> "As a small data point, though, we believed this to be qt only and couldn't hit it with rc3. It was @eklitzke's bitcoind backtrace that made it easy to reproduce."
< BlueMatt> so the flush assert(false)s because we have no "tip block" which we are flushing towards
< BlueMatt> huh? no?
< BlueMatt> I did not believe it to only be bitcoin-qt?
< BlueMatt> not sure where you got that
< wumpus> oh, and the flush assumes that the genesis block is there?
< BlueMatt> the flush assumes there is a tip block, including gensis
< promag> bitcoind crashed too
< MarcoFalke> ^
< cfields> BlueMatt: huh, I was definitely working under that impression
< BlueMatt> there are several largely-unrelated bugs in qt
< BlueMatt> and there were other init-fast-shutdown-crash bugs in bitcoind
< cfields> not anymore ofc, now that we can hit it with bitcoind.
< BlueMatt> that trying to reproduce this and other issues dug up
< cfields> right, ok
< wumpus> there's no automatic test coverage for these paths, so problems are only found incidentally
< BlueMatt> yes, one thing I did during testing was just make ShutdownRequested() start shutdown after being called N times
< BlueMatt> and just restart with an incrementing N
< BlueMatt> this is something we could automate, but would largely not have caught the qt issues
< wumpus> nice idea
< wumpus> no, testing qt is a whole different can of worms
< promag> true
< wumpus> we solved some long-running bugs there recently :)
< promag> but do we care to flush if shutdown is requested?
< cfields> BlueMatt: I like that too. We'd just want to remember to grind hard on the tests before release
< promag> (on init)
< BlueMatt> to make it actually have good coverage we need to drop all direct accesses to fShutdownRequested and replace with ShutdownRequested()
< BlueMatt> which is why I missed the current incantation of the bug
< BlueMatt> cfields: its super fast, you could run it in travis
< MarcoFalke> I can do that
< wumpus> drop all direct accesses to fShutdownRequested and replace with ShutdownRequested() -> that sounds like a good idea for encapsulation in any case
< BlueMatt> yes
< cfields> can optimize the atomic too :p
< promag> what does that solve?
< BlueMatt> anyway, as for the current bug, I think #12349 is likely fine for 0.16, though I'd prefer it skips more of the FlushStateToDisk codepaths in the case that we've clearly never written anything we need to flush (morcos just pointed out it'd be nice to skip the wallet best chain setting, though unlikely thats a bug)
< gribble> https://github.com/bitcoin/bitcoin/issues/12349 | shutdown: fix crash on shutdown with reindex-chainstate by theuni · Pull Request #12349 · bitcoin/bitcoin · GitHub
< promag> maybe we should replace atomic var with a mutex? so that some blocks can hold the mutex and prevent others to continue?
< * BlueMatt> has non-x86 hardware coming soon (I think there will likely be a dev box or two available for people who want to test/work on core on powerpc at that time as well), it'd be great to see more atomic relaxations =D
< BlueMatt> promag: wat, why would you do that?
< wumpus> looks like problem that has spooked us for a long time, that much of the code cannot cope without genesis block set
< cfields> promag: that's a different long discussion :). It was really just a joke in this context
< wumpus> it should only be a problem in shutdown() because we don't exit AppInit() succesfully in that ase
< wumpus> AppInitMain()
< BlueMatt> wumpus: yes, one other thing to be done is to move fucking ThreadImport up inside init, but regular old reindex is hard to work with to fix this issue
< BlueMatt> unless you want to re-introduce the bug where we write a copy of genesis every time we load.......
< wumpus> but other sneaky things come up all the time
< promag> BlueMatt: because once you request the shutdown other threads can start the tear down, which can mess other stuff
< BlueMatt> promag: shutdown can happen at any point, you shouldnt be able to block shutdown by taking a lock.....
< wumpus> BlueMatt: let's at least have a test for that, that if someone regresses that we'd at least detect it
< BlueMatt> subsystem separation should work.......
< BlueMatt> wumpus: yes, agreed....didnt MarcoFalke just say he'd build it? =D
< cfields> BlueMatt: yea, my earlier iterations skipped more of FlushStateToDisk, but I figured it was best to not tangle that logic up with init's
< wumpus> promag: we don't really have that issue afaik
< wumpus> but ok, then we keep #12349 for 0.16
< BlueMatt> cfields: hmm, ugh
< gribble> https://github.com/bitcoin/bitcoin/issues/12349 | shutdown: fix crash on shutdown with reindex-chainstate by theuni · Pull Request #12349 · bitcoin/bitcoin · GitHub
< BlueMatt> cfields: cant we literally skip the entire function in that case?
< BlueMatt> we clear the dbs on load themselves
< wumpus> please pray let it be fixed now
< BlueMatt> so its not like we have anything at all whatsoever to flush
< wumpus> :P
< BlueMatt> lol
< cfields> BlueMatt: yes, but that involves locking changes
< cfields> (possibly only one teeny tiny one)
< wumpus> noooooo
< BlueMatt> ugh, I still want to go back to my CChainState::fWeveDoneAnythingAtAll idea
< wumpus> we saw what happened last time we tried "locking changes" :-)
< cfields> right, hence _this_ approach :)
< BlueMatt> and just insert that and skip flush if its false
< promag> next topic suggestion #11913
< gribble> https://github.com/bitcoin/bitcoin/issues/11913 | Avoid cs_main during ReadBlockFromDisk Calls by TheBlueMatt · Pull Request #11913 · bitcoin/bitcoin · GitHub
< BlueMatt> wumpus: yea, we ended up with a super-long 0.16 release process.....sorry :(
< cfields> BlueMatt: +1 for master. Completely agree.
< wumpus> BlueMatt: it's not your fault, the code is just completely crazy in that regard
< BlueMatt> wumpus: no, I meant the validationinterface queue garbage, that had a few last-minute bugs as well
< wumpus> promag: sure
< wumpus> #topic Avoid cs_main during ReadBlockFromDisk Calls
< BlueMatt> what about it?
< wumpus> yes, what about it
< BlueMatt> I mean needs a ton of rebase due to merging of a dep, but I can do that whenever folks are ready to review
< wumpus> it's obviously a concept ack
< promag> is this going forward?
< promag> I mean, it's in priority review
< BlueMatt> I dont see why not, I mean my name's on the tin, but...
< wumpus> it's not going backward at least!
< BlueMatt> oh, you're saying I should rebase
< BlueMatt> yea, k
< BlueMatt> I mean IIRC it shoudl be reviewable if you just ignore the first few commits
< BlueMatt> which are since merged
< BlueMatt> but whatever
< promag> like that, there are others that avoid cs_main
< BlueMatt> it appears I have two things in high-priority though, which is not ok
< wumpus> if some commits are merged you can say it has been going forward
< BlueMatt> anyway, next topic?
< wumpus> any other topics?
< BlueMatt> Bitcoin!
< sipa> Bitcoin!
< achow101> encrypting wallets without restarting
< wumpus> what about it?
< cfields> wumpus: It's this new cryptocurrency...
< sipa> LOL
< wumpus> aren't bitcoins those big, golden coins with 1's and 0's on them?
< wumpus> I think they're too heavy to ever be practical
< cfields> haha
< wumpus> #topic encrypting wallets without restarting (achow101)
< achow101> relevant PR is #11678
< gribble> https://github.com/bitcoin/bitcoin/issues/11678 | [wallet] Dont shut down after encrypting the wallet by achow101 · Pull Request #11678 · bitcoin/bitcoin · GitHub
< BlueMatt> yea, you have to carry around a computer to spend them....arent those those things that take up like a whole basement and you have to hire a few people to maintain it?
< achow101> there's a really really really ugly hack in that pr to make it work from the rpcconsole
< achow101> but I'm kinda stuck on a better way to let people encrypt from the rpcconsole and not have the gui get all screwed up
< promag> achow101: with dynamic wallet loading this should be easier?
< wumpus> BlueMatt: a computer?! oh no, I heard there wil never be demand for more than 10 of them in the whole world
< achow101> promag: I don't think so
< kanzure> what is the problem with a restart?
< achow101> I noted in the dynamic reloading pr that it actually has a similar problem if you try to reload the wallet that is currently used by the gui
< wumpus> kanzure: for multiwallet it can be kind of annoying, I guess
< wumpus> kanzure: if you want to encrypt 10 wallets and need to restart 10 times
< BlueMatt> I mean, yea, question is how it gets handled in dynamic multiwallet gui, handle it the same way........
< BlueMatt> all that code is gonna have to get written one way or another
< BlueMatt> de-init'ing the whole gui wallet stuff and re-init'ing it is gonna have to exist
< achow101> BlueMatt: the thing is, dynamic multiwallet doesn't handle it at all
< BlueMatt> not that I have any opinion on *how*
< cfields> maybe an unpopular opinion, but forcing the inconvenience of a shutdown for this doesn't seem terrible to me...
< BlueMatt> achow101: well that just sounds downright broken
< kanzure> wumpus: perhaps just do it all at once and do only one restart.
< BlueMatt> cfields: see wumpus note on encrypting multiple wallets
< wumpus> cfields: I don't think it's particularly urgent either, no
< cfields> in fact, seems like it should really be handled by a tool outside of bitcoind
< wumpus> but I never encrypt wallets so don't listen to me
< BlueMatt> sounds good, lets remove encrypted wallet support!
< promag> achow101: it needs fixing then, between dyn wallet, your pr and #11383
< wumpus> (well obviously I encrypt backups, but I don't use bitcoin's encrypted wallet stuff)
< gribble> https://github.com/bitcoin/bitcoin/issues/11383 | Basic Multiwallet GUI support by luke-jr · Pull Request #11383 · bitcoin/bitcoin · GitHub
< sipa> cfields: i think that we're struggling to do so is probably a sign of some problems internally, regardless of whether it's a desired feature or not
< achow101> My plan for doing wallet encryption by default needed to do something like this, so I went and did it as a separate thing
< wumpus> but yes, once we have dynamic wallet unloading and loading, it shoudl be straightforward
< BlueMatt> sipa: that was my point...we're gonna have to handle it wrt dynamic multiwallet
< wumpus> so focus on that first
< achow101> but I kinda got stuck at how the hell to get the rpcconsole to tell the gui to reload the wallet
< sipa> BlueMatt: yup, agree
< wumpus> just unload the wallet ,resilver/rewrite it, then reopen it, voila
< cfields> sipa: fair point.
< promag> wumpus: +1
< achow101> wumpus: that's what I thought. but then qt got in the way
< promag> the gui should react
< promag> some signals etc
< wumpus> achow101: GUI should subscribe to wallet unload events
< achow101> wumpus: qt and the rpcconsole donn't interact particularly well
< achow101> apparently
< wumpus> achow101: and delete the associated state from the GUI
< wumpus> promag: yes exactly
< BlueMatt> achow101: I'm confused, so this is an open issue to solve for dynamic multiwallet gui, no?
< achow101> BlueMatt: it is
< BlueMatt> ok, so the question is how to handle it properly in that case?
< wumpus> wallets have an associated GUI object (walletmodel) which then needs to be deleted, as well as the tabs/other stuff associated with that wallet
< promag> what's the deal with rpcconsole? because of the target wallet of wallet commands?
< achow101> BlueMatt: the issue that needs to be handled is how to get the gui to reload a wallet from actions that happened over rpc
< wumpus> how is the console a problem? does it care about wallets?
< promag> I think #11383 deals with changing wallets, it's a matter to change to null wallet
< gribble> https://github.com/bitcoin/bitcoin/issues/11383 | Basic Multiwallet GUI support by luke-jr · Pull Request #11383 · bitcoin/bitcoin · GitHub
< wumpus> it just interprets commands and sends them to the RPC interface IIRC
< wumpus> oh the wallet selectbox thing
< wumpus> yes, that needs to listen to wallet removal events too, then...
< achow101> wumpus: things that happen over rpc need to be reflected in the gui
< wumpus> so I'd say first implement it in the RPC
< kanzure> are the relevant qt signal hooks not implemented..?
< wumpus> then later make a PR to do it for the GUI
< promag> both gui and rpc should load/unload wallets in the same place, and only then the gui reacts
< wumpus> that's usually the order in which we do things
< achow101> wumpus: but then people may use the rpcconsole which just does the rpc things, and that will just break the gui.
< achow101> it segfaults
< wumpus> GUI is usually a hairier business, and needs different reviewers
< wumpus> oh yes it should certainly not segfault..
< achow101> e.g. for the encrypting the wallet without restarting, it works over rpc and in the gui if you encrypt from gui
< achow101> but if you call encryptwallet from the rpcconsole, it will segfault
< achow101> because rpc cannot tell gui to reload the wallet
< wumpus> so that's because we don't have the hooks in place for dynamic wallet loading/unloading yet
< wumpus> we need that first
< achow101> yes
< promag> rpc and gui don't know each other
< achow101> I guess the question is really *how*
< achow101> since rpc and gui don't really talk to each other
< wumpus> ay other topics?
< wumpus> achow101: using signals, conencting the GUI to them
< promag> achow101: let's discuss this later
< wumpus> achow101: the same as it works for other ClientModel. WalletModel signals
< jnewbery> dynamic wallet loading (#10740) is very much a work in progress. I haven't touched it for a while because it felt like we would want multiwallet in the GUI to go in first
< gribble> https://github.com/bitcoin/bitcoin/issues/10740 | [WIP] [wallet] dynamic loading/unloading of wallets by jnewbery · Pull Request #10740 · bitcoin/bitcoin · GitHub
< wumpus> so there's a signal WalletAboutToBeUnloaded, and a signal NewWallet, the GUI can handle those and take appropriate action
< promag> jnewbery: right, that was the last conclusion iirc
< wumpus> other topics?
< kanzure> please send me your topic suggestions for my collection for march things, including short-term (like specific issues or merge requests) and long-term (future stuff)
< wumpus> #action please send kanzure your topic suggestions for my collection for march things, including short-term (like specific issues or merge requests) and long-term (future stuff)
< kanzure> i could also just randomly assign issues :-)
< cfields> kanzure: thanks for doing that
< luke-jr> #12208 should still be in 0.16, somehow not tagged/merged yet
< gribble> https://github.com/bitcoin/bitcoin/issues/12208 | GUI: Rephrase Bech32 checkbox texts, and enable it with legacy address default by luke-jr · Pull Request #12208 · bitcoin/bitcoin · GitHub
< achow101> any news on the mpc rsa signing key?
< wumpus> luke-jr: doing a string change in a rc would be really unwise, we can tag it 0.16.1 though
< luke-jr> wumpus: more unwise than having confusing strings?
< wumpus> luke-jr: yes
< wumpus> it's too late, translators have no chance to even look at it anymore
< luke-jr> could copy the translations to it, but whatever
< luke-jr> long-term, I guess that could result in confusing translations persisting XD
< cfields> I'm sure we'll learn a lot about bech32 confusion with 0.16.0. We'll be in better shape to clean it up for 0.16.1 with that feedback, I think.
< wumpus> &
< wumpus> ^
< luke-jr> well, as-is, it seems to affirm the myth that there is a from address
< * booyah> can translate whatever string to PL if anyone needs quickly just ping me
< booyah> if it's like 1 string I bet on #bitcoin + reddit community has someone ready for any lang
< achow101> meta topic: meeeting notes
< wumpus> #topic meeting notes
< achow101> the person I got to write the meeting notes got bored with it so they aren't happening anymore :(
< sipa> can't blame them, i guess..
< wumpus> oh :(
< cfields> sex.
< cfields> there, fixed. next topic?
< wumpus> heh
< wumpus> I think we're out of topics
< wumpus> #endmeeting
< lightningbot> Meeting ended Thu Feb 15 19:48:53 2018 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< kanzure> for march topics, send your topics or someone else's topics. it's fair game to insist that someone else talks about a thing >:-).
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #12442: devtools: Exclude patches from lint-whitespace (master...Mf1802-devtoolsLintWhitespaceExcludePatches) https://github.com/bitcoin/bitcoin/pull/12442
< achow101> kanzure: what's the current list of topics?
< kanzure> achow101: i have a few items, might post it in public soon. i've done this a few times in the past before meetups.
< kanzure> usually i just write down a wishlist of what i want people to talk about, but i definitely overlook some items just out of sheer ignorance or something
< kanzure> or what i know people are working on
< promag> achow101: fyi later I'm going to test 11678
< achow101> kanzure: ah, I thought the list might be published already
< achow101> promag: ok
< achow101> promag: that current implementation is really bad
< jonasschnelli> sorry. missed the meeting. currently UTC+10.5
< wumpus> jonasschnelli: you didn't miss that much, though please join in our prayer that rc4 will be final :)
< sipa> i guess it will be an alleged rc4? :p
< gmaxwell> You need to discard it's initial output if you want it to be good.
< morcos> i think thats what the c stands for
< bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/d09968f4d006...58715f6d073f
< bitcoin-git> bitcoin/master fc888bf Wladimir J. van der Laan: util: Fix multiple use of LockDirectory...
< bitcoin-git> bitcoin/master 1d4cbd2 Wladimir J. van der Laan: test: Add unit test for LockDirectory...
< bitcoin-git> bitcoin/master 58715f6 Wladimir J. van der Laan: Merge #12422: util: Make LockDirectory thread-safe, consistent, and fix OpenBSD 6.2 build...
< bitcoin-git> [bitcoin] laanwj closed pull request #12422: util: Make LockDirectory thread-safe, consistent, and fix OpenBSD 6.2 build (master...2018_01_openbsd_util_fix) https://github.com/bitcoin/bitcoin/pull/12422
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/58715f6d073f...5eff1c748d56
< bitcoin-git> bitcoin/master ceaefdd Cory Fields: fix possible shutdown assertion with -reindex-shutdown...
< bitcoin-git> bitcoin/master 5eff1c7 Wladimir J. van der Laan: Merge #12349: shutdown: fix crash on shutdown with reindex-chainstate...
< bitcoin-git> [bitcoin] laanwj closed pull request #12349: shutdown: fix crash on shutdown with reindex-chainstate (master...fix-qt-shutdown) https://github.com/bitcoin/bitcoin/pull/12349
< promag> achow101: nack on those __Model::ReloadWallet()
< promag> by design an __Model instance is tied to the wallet received in the constructor
< promag> the same for WalletModel
< achow101> promag: yeah, I'll probably ditch all of that once I figure out how to use boost signals
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/5eff1c748d56...3fa556aee203
< bitcoin-git> bitcoin/master 5f605e1 Pieter Wuille: Make signrawtransaction accept P2SH-P2WSH redeemscripts
< bitcoin-git> bitcoin/master 3fa556a Wladimir J. van der Laan: Merge #12427: Make signrawtransaction accept P2SH-P2WSH redeemscripts...
< promag> I really think you should wait for jnewbery or try to rebase his PR and build from there
< bitcoin-git> [bitcoin] laanwj closed pull request #12427: Make signrawtransaction accept P2SH-P2WSH redeemscripts (master...201802_signrawp2shp2wsh) https://github.com/bitcoin/bitcoin/pull/12427
< promag> you should consider TransactionTableModel::wallet const (the same for other models)
< bitcoin-git> [bitcoin] laanwj pushed 4 new commits to 0.16: https://github.com/bitcoin/bitcoin/compare/5e40e64face2...3762ac127aec
< bitcoin-git> bitcoin/0.16 32a7268 Wladimir J. van der Laan: util: Fix multiple use of LockDirectory...
< bitcoin-git> bitcoin/0.16 4d54e7a Wladimir J. van der Laan: test: Add unit test for LockDirectory...
< bitcoin-git> bitcoin/0.16 ad10b90 Cory Fields: fix possible shutdown assertion with -reindex-shutdown...
< promag> wumpus: if you are up to merges see #12083
< gribble> https://github.com/bitcoin/bitcoin/issues/12083 | Improve getchaintxstats test coverage by promag · Pull Request #12083 · bitcoin/bitcoin · GitHub
< wumpus> sorry, I'm busy at the moment
< bitcoin-git> [bitcoin] laanwj pushed 1 new commit to 0.16: https://github.com/bitcoin/bitcoin/commit/51093478c0e1d79b2aa4296c5f93dcf41c1ea92f
< bitcoin-git> bitcoin/0.16 5109347 Wladimir J. van der Laan: qt: Pre-rc4 translations update...
< promag> np, it's mainly a test pr with 2 utack
< cfields> wumpus: preparing to tag tonight?
< wumpus> cfields: yep
< cfields> great :)
< sipa> "the same thing we do every night, pinky"
< wumpus> pinky and the brain brain brain
< bitcoin-git> [bitcoin] laanwj pushed 1 new commit to 0.16: https://github.com/bitcoin/bitcoin/commit/a8e62a842615d338d41caf1603de4263a7f4bad8
< bitcoin-git> bitcoin/0.16 a8e62a8 Wladimir J. van der Laan: doc: Update release notes from wiki for rc3...
< bitcoin-git> [bitcoin] theuni opened pull request #12444: gitian: bump descriptors for (0.)17 (master...gitian-bump) https://github.com/bitcoin/bitcoin/pull/12444
< cfields> haha
< wumpus> * [new tag] v0.16.0rc4 -> v0.16.0rc4
< cfields> woohoo
< cfields> now go relax :)
< wumpus> :)
< * esotericnonsense> cheers
< meshcollider> Oh I missed the meeting
< meshcollider> rc4 \o/
< dx25> @sipa, did you write the cashaddr.h/cpp module in bitcoin-abc?
< dx25> because if so, that's hilarious.
< sipa> no, but it was adapted from the bech32.cpp module in bitcoin core
< sipa> which i did write
< dx25> oh ok