< 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.
< 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.
< 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
< 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
< 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
< 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?
< 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.
< 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
< 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.
< 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)
< 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
< 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
< 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
< 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
< 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.
< 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