< sdaftuar>
i'm happy to simulate it for you on the same data set i've been using, just tell me what parameters you'd like to see
< sdaftuar>
i'm not a fan of the model though
< gmaxwell>
sdaftuar: 10 seconds and .0005 BTC should be fine for a test run.
< gmaxwell>
sdaftuar: I would gladly agree that it's a hack. But -- it's surely a simple one, if there is nothing else that could be said about it.. CreateNewBlock performance and simplicity counts for a lot.
< gmaxwell>
sdaftuar: also because of how BIP152 works it is still preferable to have less new data. E.g. having a binary "contains new stuff" vs "not" isn't ideal--- because once prefilling is in use up to 10kb will be prefilled and if there is more than that, we're likely to prefill noting (under the assumption that the block is too inconsistent to be salvagable)
< sdaftuar>
gmaxwell: that seems gameable though? if i just rbf a transaction every 10 seconds with fee 50000, 50000+incremental_relay_fee, ... you'll always produce a block with a recent transaction, and i'll give up a negligible amount of money?
< gmaxwell>
sdaftuar: 10 seconds is a non-trivial amount of time to allow propagation however.
< sdaftuar>
gmaxwell: i agree about the prefilling concern... that was part of the reason i was skeptical of this approach at all, but i figured that for now, "needs a roundtrip" is pretty different from not needing a roundtrip, so might as well capture that.
< sdaftuar>
even if i rbf every 5 seconds, its a trivial amount of money
< gmaxwell>
I did calculations before that were hand-wave-handwave about the cost of orphaning relative to the subsidy based on block propagation observations and concluded a pretty low fee pays for the risk.
< gmaxwell>
(mentioned somewhere in the logs here)
< sdaftuar>
yeah i have no idea what the actual stale rates people see are
< sdaftuar>
(and really, my model requires converting two numbers into 1 -- stale rate without recent txs/stale rate with recent txs -> my parameter. not a difficult calculation, but requires understanding
< sdaftuar>
for clarity: that "/" above is not meant to be division
< gmaxwell>
well my figuring didn't use stale rates but measurements of delays between stratum updates at pools as a function of blocksize, so I was able to come up with a constant + factor*size model for orphaning risk. This was before BIP152-- and I think we can't really measure it anymore, but I figure the before bip152 figures give a baseline for the marginal impact. my model basically assumed the comp
< gmaxwell>
etition was sending an empty block.
< sdaftuar>
gmaxwell: for the 0.14.1 release notes, did you want to specifically mention that the 0.14.0 defaults could result in the utxo cache using 1.2GB of memory due to mempool sharing?
< sdaftuar>
i just pushed an update to #10185 but didn't call out that fact specifically, let me know if you'd like me to add it
< gmaxwell>
sdaftuar: I don't remember now, I don't think I wanted to do that. did I? uh. I don't think we should bother mentioning the 0.14.0 usage. Did you mention it before but only say it could use 600 or something?
< sdaftuar>
gmaxwell: i just explained that the 300MiB default could result in 600MiB of usage, so that people would know to double their values if they wanted to, say, keep the whole thing cached still
< sdaftuar>
ah, i guess my language is potentially confusing
< gmaxwell>
sdaftuar: but with the mempool the defaults could be 1200 which is what I was trying to express.. the dbcache just get doubled, the mempool did too.
< sdaftuar>
right
< sdaftuar>
all i wanted to explain was "multiply by 2!"
< sdaftuar>
maybe just delete my parenthetical?
< sdaftuar>
ok, got rid of it -- hopefully the sentence about only accounting for half the peak utilization will be enough for people to figure out what they want to do.
< wumpus>
"oh dear god lets not default to running random peoples' commitmsg scripts on wumpus' computer" yes that's not going to happen, I'm already paranoid with github-merge.py itself, I use a version outside the repository and only update it if I carefully reviewed changes inside the repo
< gmaxwell>
lol, please only on pulltester which is already vulnerable as heck.
< MarcoFalke>
The script could be run in a vm. Though, it comes with an overhead...
< gmaxwell>
There are a LOT of vm escape exploits.
< wumpus>
in any case I don't like the idea of adding more functionality to github-merge.py, please just keep it a merge script and not add any other hooks
< MarcoFalke>
Running it on travis is not enough. travis is just an indicator that should not be trusted
< sipa>
i doubt you can trigger any of them from a shell script in a way that it's obvious to cursory review, though
< gmaxwell>
most recent one in QEMU was awful, with the instruction decoder. Esp since previously I'd intentionally avoided kvm to try to lower risk but that made it trivially exploitable.
< MarcoFalke>
Just need to make sure the script is reviewed on every (ut)ACK
< gmaxwell>
sipa: doubt it enough to put a bounty on it though? :P
< bitcoin-git>
bitcoin/master e78bc45 NicolasDorier: [Wallet] Decouple CInputCoin from CWalletTx
< bitcoin-git>
bitcoin/master f597dcb NicolasDorier: [Wallet] Simplify code using CInputCoin
< bitcoin-git>
[bitcoin] laanwj closed pull request #10165: [Wallet] Refactoring by using CInputCoin instead of std::pair (master...inputcoin) https://github.com/bitcoin/bitcoin/pull/10165
< wumpus>
gmaxwell: one problem is that x86 is such a complex beast to emulate (or even write a instruction decoder for). If you induce the overhead of full emulation for security reasons, might as wll choose a simpler platform to emulate
< wumpus>
may also help to restrict what qemu can do through e.g. apparmor, so if a process manages to escape from the vm it isn't immediately able to do much more than the VM could. libvirt does that by default AFAIK
< sdaftuar>
gmaxwell: i ran the sim, with 10 seconds / 50000 satoshi fee threshold, roughly 95% of CNB invocations included a recent transaction.
< sdaftuar>
gmaxwell: for comparison, i re-ran my patch with a setting of 10seconds, 0.2% fee drop, and less than 2% of CNB calls included recent transactions
< bitcoin-git>
[bitcoin] mariodian opened pull request #10201: pass Consensus::Params& to ReceivedBlockTransactions() (master...consensusparams-receivedblocktransactions) https://github.com/bitcoin/bitcoin/pull/10201
< bitcoin-git>
[bitcoin] NicolasDorier opened pull request #10202: [Wallet] FundRawTransaction can fund a transaction with preset inputs found in the CoinView (master...fundrawtransaction) https://github.com/bitcoin/bitcoin/pull/10202
< BlueMatt>
cfields: hmm, whats the status of #7522?
< gribble>
https://github.com/bitcoin/bitcoin/issues/7522 | Bugfix: Only use git for build info if the repository is actually the right one by luke-jr · Pull Request #7522 · bitcoin/bitcoin · GitHub
< BlueMatt>
luke-jr: are you going to rebase #7289?
< wumpus>
I agree it's a bit strange but I can't be bothered to change it, it's not as if incidents ever get reported to it
< wumpus>
eh, wrong key
< BlueMatt>
wumpus: you have a key which types whole sentences? wow!
< BlueMatt>
:p
< wumpus>
yes this smart-keyboard has analyzed all of the text I've entered on it, put it into a markov chain, and now generated it's own text with just one keypress :p
< BlueMatt>
wumpus: you jest, but thats what fucking mobile phones do :(
< jnewbery>
and your markov chain predicts that your most common response is "I agree it's a bit strange but I can't be bothered to change it". Seems pretty accurate :)
< wumpus>
ah yes that's always the first thing I disable
< wumpus>
jnewbery: yep, the keyboard can almost replace me already :-)
< michagogo>
Yeah, the keyboard auto correct is the best app that I can use on my iPhone and my phone is a very quiet app and the fact is that it doesn't have a plan to use Facebook to access the web for the past couple weeks
< michagogo>
(I typed the first 4 words of that and selected the rest from the 3 suggestions)
< sipa>
wumpus: it's annoying to create PRs against the bitcoin core leveldb repository
< sipa>
because it's marked as 'forked from' the upstream google repo (which didn't exist at the time ours was created)
< sipa>
*isn't marked
< sipa>
seems the only way to fix that is to delete it and recreate it
< wumpus>
that'll lose all the current issues and PRs though
< sipa>
true
< sipa>
maybe we can contact support to just fix it for us?
< wumpus>
yes I think they can reparent repositories
< sdaftuar>
sipa: i was trying to review #9743, and i'm confused by the lockstack cleanup commit. the boost documentation i'm reading suggests that by default, delete should be called already?
< sdaftuar>
there is this comment: "Note: on some platforms, cleanup of thread-specific data is not performed for threads created with the platform's native API. On those platforms such cleanup is only done for threads that are started with boost::thread unless boost::on_thread_exit() is called manually from that thread.
< sipa>
sdaftuar: but that would mean that the custom cleanup function is also not called?
< sdaftuar>
sipa: that was my guess, but i wasn't sure!
< cfields_>
BlueMatt: hmm, i thought 7522 was merged a long time ago. Having a look.
< luke-jr>
hm, thought I was late; am I early?
< gmaxwell>
you are early
< luke-jr>
cool
< aantonop>
gmaxwell: apologies for the trolling by an "aantonop" imposter the other day. I hadn't turned on Nickserv enforcement. It's on now.
< gmaxwell>
aantonop: oh no problem, people should have just ignored him. :) trolls show up all the time.
< spudowiar>
Is there a good way to get a transaction input and find the BIP32 derivation path?
< spudowiar>
I don't think the keystore stores BIP32 derivation information
< luke-jr>
pretty sure it does..
< spudowiar>
luke-jr: Well, I was looking in src/keystore.h but it seems I can only get a CKey or CPubKey out which I don't think stores derivation information
< sipa>
CKeyMetaData or something stores ot
< sipa>
*it
< spudowiar>
sipa: How do I get one of those from a public key?
< wumpus>
meeting time?
< jonasschnelli>
Yes. Hi all.
< wumpus>
#startmeeting
< lightningbot>
Meeting started Thu Apr 13 19:00:17 2017 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
< cfields_>
yes, let's finish discussing after the meeting
< cfields_>
ok, for reference: #10189
< gribble>
https://github.com/bitcoin/bitcoin/issues/10189 | devtools/net: add a verifier for scriptable changes. Use it to make CNode::id private. by theuni · Pull Request #10189 · bitcoin/bitcoin · GitHub
< luke-jr>
mv meeting/* after-meeting/
<@wumpus>
haven't had a time to look at that one yet
< cfields_>
the idea is for changes that are basically just search/replace, to allow a short script to be placed in the commit message, and verified by Travis
< cfields_>
if they don't transform the old commit to the new one exactly, travis will reject
< cfields_>
the aim is to make those kinds of changes easier to review
< BlueMatt>
is ther emuch to discuss? I think its a good idea
< gmaxwell>
I'm fine with it so long as we don't expect commiters to be running it. I think someone should write a list of examples, because when I've made mass programatic changes I haven't done it in ways that would look too tidy in a commit message.
<@wumpus>
would that execute arbitrary scripts in commit messages?
< luke-jr>
which is basicaly what you're suggesting)
< cfields_>
BlueMatt: well, you brought up a good point about the safety of random scripts
<@wumpus>
seems kind of dangerous, I certainly wouldn't want to run that locally
< cfields_>
so i thought it might be worth discussing possibly constraining. Say to sed or so.
< BlueMatt>
gmaxwell: there are already two examplesa
<@wumpus>
I'm really careful what scripts I run, and wouldn't want anything to be run automatically
< BlueMatt>
and, yes, agree, jtimon suggested that they only be run if there is a scripted prefix in the commit's title
< spudowiar>
What sort of damage can be done with, say, sed or awk?
< BlueMatt>
to make it much more obvious
< petertodd>
wumpus: granted, if you ever compile bitcoin you're running stuff within the repo anyway
< gmaxwell>
spudowiar: arbritary damage.
< spudowiar>
I don't think you can do much with sed, not too sure about awk
< spudowiar>
gmaxwell: I mean dangerous code execution
< cfields_>
wumpus: sure, agreed. Travis can run them, and they can ofc be run manually as well.
< jcorgan>
maybe there should only be a predefined set of opcodes in the script that can run, with a stack...oh wait
<@wumpus>
just make it create a ~/.bashrc, runs next time
< gmaxwell>
in any case, if it's a travis mostly thing I think thats ducky. I would attempt to use it.
<@wumpus>
I'd really prefer not to do this
< luke-jr>
I don't see the value in a Travis-"mostly" thing.
< spudowiar>
What are the benefits?
< gmaxwell>
I guess the reason to just not commit the script is that they're one time things?
< cfields_>
gmaxwell: that was my intention, yes.
< luke-jr>
it gives a false sense of review
<@wumpus>
if there's a script then reviewers can manually verify it too, after reviewing the script
< BlueMatt>
gmaxwell: yes, if they're one-time things its annoying to commit them, also annoying to lose them
< sipa>
when would these scripts be ru
< sipa>
n
< cfields_>
yes, i got the idea from bac5c9cf643e9333479ac667426d0b70f8f3aa7f
< luke-jr>
I wouldn't mind including the scripts in non-first-line commit msg, and having a dev tool to run them, but IMO better if Travis *doesn't* do it
< gmaxwell>
luke-jr: but reviewers can test it. After reviewing the script. The challenge there is just that you normally don't critically review commit messages in that way. but it's kind of perfect-- in that its one time information about the change.
< gmaxwell>
luke-jr: the purpose of travis doing it is for feedback that you've formated it right and it runs on computers other than yours. Not as a review step.
< cfields_>
if we're going to include a transform in the message like that, I figured we may as well standardize it
< luke-jr>
gmaxwell: as long as reviewers do test it, and don't trust Travis
<@wumpus>
cfields_: the idea of that was to give reviewers a way to verify it, as well as make it easy for myself to repeat the work if it needs rebase
< spudowiar>
You could create format like `*.cpp *.h | s/boost::filesystem/fs/g`
< sipa>
spudowiar: little bobby tables will haunt you
< luke-jr>
lol
< petertodd>
gmaxwell: note how this is rather like how a merkle sum tree works... the script is a function to transform input to output, and both input and final output are committed via the git commit object
< cfields_>
ok, put a different way, then: if it could be done so that it worked on nothing but a dummy git index, with no access to the filesystem, would that be more acceptable?
<@wumpus>
cfields_: if a transformation of the files could be perfectly sandboxed , I'd be all for it, but I'm kind of afraid of anything that will automatically execute scripts from commit messages, especially as most people review the code changes but not so much the messages :)
< petertodd>
cfields_: I mean, with travis the compilation process can do anything anyway, so I don't see how this is a security risk on top of anything else
< gmaxwell>
I don't know that sandboxing this is worth the effort. If you want to do that, knock yourself out, I guess?
< petertodd>
cfields_: that's true for your local dev setup too
<@wumpus>
I don't think it's worth the effort, no
< BlueMatt>
wumpus: that was my concern, hence why I like jtimon's suggestion of only doing it if, eg, the commit title starts with "AUTO-SCRIPT: "
< BlueMatt>
or something
< gmaxwell>
petertodd: the only concern I have wrt security is that there is a new thing you need to review before running things. Not just the git diff but the commit messages.
< BlueMatt>
then its obvious from the first line of the commit
< cfields_>
petertodd: i agree. The concern here (I guess) is that we'd get a malicious script committed, then someone would run it locally.
< BlueMatt>
and you will see it
< petertodd>
cfields_: but that's already a problem anyway: I can commit a malicious commit that adds rm -rf / to the makefile
< gmaxwell>
cfields_: I dunno about your workflow, but I could be tricked by this, because I'll pull and git diff <where I was before> which won't show me the commit message(s).
< gmaxwell>
otherwise I'd argue there is no change.
< cfields_>
gmaxwell: but it wouldn't touch anything in that case. You'd have to actively run it.
<@wumpus>
petertodd: you can, but changes to the makefile would be apparent to me at least
< petertodd>
gmaxwell: so long as devs never have git hooks that run this automatically I think we're covered
< spudowiar>
Why not have a script that manually does these checks, prints out each commit message beginning with "AUTO-SCRIPT: " and the commands it would run and asks if you want to run them?
< petertodd>
wumpus: right, but see my reply to gmaxwell above
< gmaxwell>
petertodd: yea, I'm not too concerned.
<@wumpus>
yes, as long as it's not executed automatically outside of travis it's fine
<@wumpus>
I don't care what is done in travis, that is already somebody else's sandbox
< cfields_>
yes, that's the case as-is.
< petertodd>
wumpus: +1
< gmaxwell>
oh actually too spudowiar's suggestion is good unless run with --force the script could ask for confirmation.
< cfields_>
nothing is touched locally.
<@wumpus>
my point was just that I just don't want e.g. github_merge.py to execute it automatically
<@wumpus>
great
< petertodd>
wumpus: yup, that'd be a bit crazy :)
< cfields_>
wait, please back up, I'm not sure if there's a communication breakdown here.
< * sipa>
makes backup
< cfields_>
as-is, this is travis-only. Nothing runs automatically anywhere but there.
<@wumpus>
cfields_: as said I haven't seen the actual PR, just rumors from this channel
< spudowiar>
Well, Travis can run it with --force, github_merge.py can run it without --force?
<@wumpus>
yes makes sense to me then cfields_
< spudowiar>
Without --force, no actual commands are run
< spudowiar>
Unless you interactively say so
< gmaxwell>
spudowiar: no need to make merge run it at all. just an extra review step available.
< cfields_>
spudowiar: there's nothing to run "it".
<@wumpus>
no, github_merge.py won't run it at all, that is a merge script and shouldn't do anything else
<@wumpus>
(I run that on the machine that has the gpg key, so I'm paranoid about it)
< gmaxwell>
But I do think it would be nice if when you manually run that review step it shows you what it's going to run.. to avoid my workflow issue example.
< spudowiar>
cfields_: I'm talking about a hypothetical script
< spudowiar>
wumpus: Buy a PGP smartcard :)
< petertodd>
spudowiar: adversary still can cause the PGP smartcard to sign something you didn't aprove
<@wumpus>
ok. seems a communication breakdown here.
<@wumpus>
let's just review the PR, and talk about it again next week
< morcos>
good thing our trolling coordination doesn't suffer from these communication problems
<@wumpus>
other topics?
<@wumpus>
#action review #10189
< gribble>
https://github.com/bitcoin/bitcoin/issues/10189 | devtools/net: add a verifier for scriptable changes. Use it to make CNode::id private. by theuni · Pull Request #10189 · bitcoin/bitcoin · GitHub
< sdaftuar>
topic suggestion: goals for 0.15
< cfields_>
good one.
< BlueMatt>
ok, so ryanofsky wanted to talk about multi-process, too, i think
< BlueMatt>
and, ofc, pr review targets for the week
< BlueMatt>
:)
<@wumpus>
#topic goals for 0.15
< BlueMatt>
sdaftuar: my interpretation of "goals for 0.15" is "everyone has their own goals, and should mention the PRs they want reviewed to further their goals as review priorities, and we should all help them :)"
< gmaxwell>
Per-txo dbcache and non-atomic flushing please...
<@wumpus>
agree with BlueMatt, though people could state what their goals for 0.15 are
< jonasschnelli>
My goals for 0.15 are: HD auto-restore, Qt feebumper and multiwallet
< spudowiar>
Watch only HD wallets sound good for 0.15 :)
< gmaxwell>
and multiwallet.
< jonasschnelli>
Oh.. yes. That!
< sdaftuar>
gmaxwell: agreed, also i think it would be good to get the new fee estimation in place
< jonasschnelli>
(hd watch only)
< BlueMatt>
multithreaded net_processing (and wallet) with CValidationInterface generating callbacks into it
< BlueMatt>
ie disconnecting validation and everything else with CValidationInterface in betwene :)
< sipa>
when is 0.15 feature freeze?
<@wumpus>
luke-jr: are you planning to update the multiwallet prepare pull according to review comments, or should I take it over?
< achow101>
is replacing accounts with labels planned for 0.15?
< gmaxwell>
sdaftuar: I was going to bring that up. I think that we really need a high level description of the algorithim that we could give to non-developers (e.g. academics) to review. I don't know if that should hold the improvements but we need to get to that.
<@wumpus>
luke-jr: it's kind of blocking other things right now I think
< morcos>
yes, re: fee estimation.. i understand its a giant pain in the ass to review... and for little perceived gain. but i do think its worth it, and merging it sooner rather than later is better in case there are any edge case improvements needed.
< luke-jr>
wumpus: when I get home
< BlueMatt>
I do NOT think its little perceived gain
< cfields_>
my big goals are: finally move to libevent for connman, deterministic toolchain creator (so we can drop our reliance on ubuntu's toolchain)
< BlueMatt>
morcos: one of the topics discussed at the wallet dev meetup thing was about how shit fee estimates across the ecosystem are
<@wumpus>
yes libevent please
< BlueMatt>
and bitcoin core is a big part of that
< morcos>
gmaxwell: heh... it's more art than science i think
< gmaxwell>
jonasschnelli: I don't know about HD watch only, we have a lot of overhang in the HD change that isn't done yet. We really need to hammer out all these corner cases before we go adding more major new features in key management.
<@wumpus>
I'd also really like to get the UNIX p2p/rpc stuff in
< jonasschnelli>
gmaxwell: can you explain "a lot"?
< BlueMatt>
morcos: your previous warrants to me about weekend fee estimates in your new design being good represent a huge win for the ecosystem
< luke-jr>
wumpus: (tomorrow)
<@wumpus>
it's all very low-risk and shouldn't affect non-unix-socket use cases
< sdaftuar>
BlueMatt: I also think the ability to return non-conservative (ie, lower) fee estimates is important
< gmaxwell>
jonasschnelli: all the things related to recovery are broken and we even don't know how to fix some of them.
< morcos>
BlueMatt: yes, but exactly the kind of thing that need real world experimentation, b/c they change the real world conditions
<@wumpus>
luke-jr: thanks
< sipa>
i want pertxoutcache, remove memory peak at flushing, better dbcache eviction policy, ...
< jonasschnelli>
gmaxwell: Yes. I'm working on the recover (one of my 0.15 goals: "HD auto-restore")
< BlueMatt>
OKOK, so review priorities towards all these amazing things? whats up this week :)
< sipa>
oh, and segwit activated? pretty please?
< gmaxwell>
morcos: right but there are incentives and security implications in the design, that deserve wider review than we'll get from people who will infer it from the code. Even if the description wasn't completely precise it would help.
< BlueMatt>
sipa: lol
< cfields_>
wumpus: yes. I've looked over that and it kinda clashes with my libevent changes. But it makes sense to get yours in first, then I can adapt mine.
< * BlueMatt>
registers #10179
< gribble>
https://github.com/bitcoin/bitcoin/issues/10179 | Give CValidationInterface Support for calling notifications on the CScheduler Thread by TheBlueMatt · Pull Request #10179 · bitcoin/bitcoin · GitHub
<@wumpus>
cfields_: the RPC pull (the first one) shouldn't collide
< gmaxwell>
sipa: would likely help to get 0.14.1 out...
<@wumpus>
cfields_: I'm fine with including that one only for now, it's most of the code
< sipa>
gmaxwell: ack
< cfields_>
wumpus: no, the other. But it's a non-issue either way.
<@wumpus>
cfields_: the p2p-over-unix-socket is a very small patch on top of that, Im fine with doing that after libevent which means it can share the code with the RPC path
< morcos>
gmaxwell: yes, writing up the high level design is actually simpler than explaining the actual implementation. the high level design is pretty simple.. maybe i'll put more effort into the last commit which adds commentary
<@wumpus>
cfields_: though it depends on the time frame I guess
< gmaxwell>
morcos: I think that would be really helpful.
< cfields_>
sounds good.
< gmaxwell>
morcos: Also for general review, I've lost track of the overall design of the estimator.
< sipa>
morcos: i would like to see a 1-page document explaining what it tries to accomplish
< gmaxwell>
morcos: in any case I strongly support improving it, the current one is dumb on weekends.
< cfields_>
sipa: let's activate segwit after the meeting. We only have 20 min left :p
< gmaxwell>
cfields_: ack
<@wumpus>
#action activate segwit
< morcos>
sipa: ok
< gmaxwell>
jinx
< sipa>
(i have no clue how the estimator works, or ever worked, beside "something with buckets")
< morcos>
gmaxwell: yes this is much better on weekends (optionally)
< instagibbs>
ack on explanation, I was asking really dumb questions about it yesterday, may help
< gmaxwell>
So we should bring up again per-txo and non-atomic flushing. These changes are straight forward but call for a lot of heroic crash testing.
< BlueMatt>
gmaxwell: I'm waiting for non-atomic to support disconnects
< sipa>
gmaxwell: agree, it needs to support reorgs
< sipa>
also... even harder to test
< cfields_>
high-level question about per-txo: what's the desired/expected outcome when downgrading from 0.15 to 0.14 ?
< gmaxwell>
sipa: hm? with the current code we finish the flush all at once.
< gmaxwell>
needing to support reorgs is only an issue with the changes we discussed post per-txo.
< MarcoFalke>
wumpus: The pull jnewbery just opened could go into 0.14.1 if we do another rc anyway
< sipa>
gmaxwell: a crash in the middle a flush after a disconnect cannot be recovered from with the current code
<@wumpus>
MarcoFalke: yep
< gmaxwell>
cfields_: per-txo cannot be downgraded. reindex. Gotta take the cost at some point. Non-atomic is downgradable on clean shutdown, reindex otherwise.
< jnewbery>
MarcoFalke has suggested that I split off the non-backwards compatible rpc argument name change from #10143 into its own PR so it can be backported
< gmaxwell>
cfields_: We're going to have to take a non-downgradable change for per-txo at some point, I don't think never doing it is a realistic option, the improvement is too significant.
<@wumpus>
jnewbery: makes sense, it's still fairly safe to rename named arguments now, but they should be backported
< gmaxwell>
cfields_: the overall changes to the dbcache are too burdensom to realisitcally support a version that can support both.
< sipa>
i still wish for non-atomic flush in 0.14.2
< cfields_>
gmaxwell: sure, makes sense. is it worth trying to slip in a bit of smarts into 0.14.1/0.14.2 gracefully saying that (in the event of an attempted downgrade) the format has changed?
< * sipa>
keeps dreaming
< gmaxwell>
cfields_: yes, that would be more reasonable than getting a generic corruption message.
<@wumpus>
cfields_: yes, something for 0.14.2
< sipa>
cfields_: we could add a change in 0.14 that would make explicit downgrade easy
< morcos>
sipa: too big a change
<@wumpus>
no more new things for 0.14.1 please
< cfields_>
.2, ignore for .1.
< gmaxwell>
"Your database format is from the future, so sad for you. Please run reindex."
< cfields_>
wumpus: sorry, realized my mistake too late :)
< sipa>
morcos: i said wish, not demand :)
< sdaftuar>
sipa: feature freeze for 0.15 is 2017-07-16
<@wumpus>
apparently we need to make meetings invite-to-speak only
<@wumpus>
stupid childish trolls are why we can't have nice things
< kanzure>
you used wrong ip address last time
< spudowiar>
wumpus: If you did that, I wouldn't be able to participate :(
< gmaxwell>
wumpus: lets discuss in the meta meeting. :P
< luke-jr_>
spudowiar: Well, no one gives a fuck
< cfields_>
kanzure: no need, they're easy to spot. just ignore.
< gmaxwell>
(after the meeting)
< gmaxwell>
in any case, lots of good things in flight. I think progress for 15 is good right now.
< BlueMatt>
ok, so 10 minutes left
< BlueMatt>
ryanofsky: ?
< BlueMatt>
did you want to talk about multi-proces
< BlueMatt>
also please folks list your review-priority for this week :)
< spudowiar>
If that gets merged I will hug ryanofsky :)
<@wumpus>
spudowiar: I don't see why not; would just make it more work to invite everyone
< ryanofsky>
not sure, what there is to say, #10102 is not complete, but the code that's there could use some review
< gribble>
https://github.com/bitcoin/bitcoin/issues/10102 | bitcoin-qt: spawn bitcoind and communicate over pipe (Experimental, WIP) by ryanofsky · Pull Request #10102 · bitcoin/bitcoin · GitHub
< spudowiar>
wumpus: No one would invite me :(
< sipa>
i'm not sure what the appeal of multi process is
<@wumpus>
#topic multiprocess
<@wumpus>
process isolation
< sipa>
i'd like to see the wallet go into a separate process from the networking
< jonasschnelli>
sipa: #10102
< gribble>
https://github.com/bitcoin/bitcoin/issues/10102 | bitcoin-qt: spawn bitcoind and communicate over pipe (Experimental, WIP) by ryanofsky · Pull Request #10102 · bitcoin/bitcoin · GitHub
< BlueMatt>
sipa: then review my PRs, those do the first step of splitting it off neatly
< cfields_>
ryanofsky: I've been wanting to split out net into a separate process, but the barrier was creating an IPC. So I'm all for it.
< BlueMatt>
then we can use ryanofsky's multiprocess stuff to do the process :)
<@wumpus>
not having the gui and the rest in one address space would be a good start
< sipa>
that that's qt from the rest
< sipa>
that seems like a pointless gimmick to me
< jonasschnelli>
Yes. The wallet seems to be more important.
<@wumpus>
sigh
< gmaxwell>
If it could disconect and reconnect it would have some value.
< ryanofsky>
agreed the wallet is more important
< * BlueMatt>
wasnt sure whether folks would like the many-process approach, over two-processes (wallet, and other)
< jonasschnelli>
not saying the Qt part is not important
< ryanofsky>
reason for starting with qt is that wallet work will depend on matt's changes
< jnewbery>
I'd like one process per wallet for multi-wallet
< spudowiar>
Oh, I thought that was wallet as well (my offer of hug has been withdrawn, ryanofsky)
<@wumpus>
the gui has more pressing problems though: too much happens synchronous with the core in the GUI loop
< jonasschnelli>
wumpus: thats a good point
< ryanofsky>
and qt seemed like it might be a less controversial place to start since we already have separate bitcoind and bitcoin-qt executables
<@wumpus>
ryanofsky: yes I agree
< cfields_>
i suspect this will probably go down like the scripted-diffs discussion. Maybe we should assign ourselves homework to read the PR fully and discuss next week?
< jonasschnelli>
Architectural wise: splitting of Qt makes more sense.. security: wallet. Splitting of the wallet with something similar to an ssh/gpg-agent shouldn't be very complicated.
<@wumpus>
ryanofsky: would make sense to not have them share all that code
< sipa>
hmm, i withdraw my comment about it being pointless
< gmaxwell>
I've read the PR.
<@wumpus>
also having one example of IPC between process could make other splits easier
< gmaxwell>
But this is not a good design.
< gmaxwell>
as such an example.
< jnewbery>
5 minutes left.
< jnewbery>
Suggested topic: PR review requests
<@wumpus>
gmaxwell: have you commented about the design in the PR?
<@wumpus>
#topic high-priority PR review requests
< BlueMatt>
#10179
< gribble>
https://github.com/bitcoin/bitcoin/issues/10179 | Give CValidationInterface Support for calling notifications on the CScheduler Thread by TheBlueMatt · Pull Request #10179 · bitcoin/bitcoin · GitHub
< spudowiar>
wumpus: For external signing, currently messing about with external command, using stdio but other IPC example would be useful
< spudowiar>
Oops, topic's moved on, sorry
< gmaxwell>
(What this PR does is pulls in a compat library to just break the function boundaries, to work across processes, it is not an actual IPC. And for just making the GUI more concurrent and perhaps reconnectable that fine. But that is not how we should seperate the wallet.
< gmaxwell>
)
< gmaxwell>
wumpus: I can comment more if I haven't.
< spudowiar>
Wallet communicating over the JSON-RPC interface would be quite good, but then you have the issue that wallet functionality is provided over the JSON-RPC interface?
< BlueMatt>
gmaxwell: so is your objection that qt and other things dont need separated, or that this isnt the right approach?
< spudowiar>
luke-jr: That's the point I was making. If you have the wallet talking to the daemon over JSON-RPC, how can you provide the wallet functionality over JSON-RPC?
< spudowiar>
luke-jr: I'm just really bad at wording stuff :)
< jnewbery>
cfields_ wumpus thanks
< spudowiar>
petertodd: That's why we need OpenPGP Card 4.0 :)
< spudowiar>
petertodd: So that the smart card can request bits of the data, etc. and display it on the display
< petertodd>
spudowiar: it'll have to have a display at least... fudnemental probelm
< spudowiar>
petertodd: TREZOR
< spudowiar>
petertodd: I have a working OpenPGP implementation for TREZOR
< spudowiar>
Only does ECDSA signing though
< spudowiar>
Rewriting it in Rust though
< petertodd>
spudowiar: heh, git support for trezor...
< spudowiar>
petertodd: I signed a Git commit with it :)
< petertodd>
spudowiar: oh, theres a rust impl for the trezor uc?
< gmaxwell>
BlueMatt: QT being seperated can improve concurrency and ... if done right allow it to turn off and on, while the daemon keeps running. Which I think are moderately useful things. And the way that it is done here is more or less okay for just accomplishing that. But it not a way that we should seperate the wallet, it is HIGHLY trusting, it wouldn't support multiple concurrent users, it doesn'
< spudowiar>
petertodd: Rust runs on pretty much anything (there's an LLVM backend for STM32*)
< gmaxwell>
t provide for a stable interface. etc.
< petertodd>
spudowiar: nice!
< luke-jr>
bbl
< gmaxwell>
wumpus: metametting we should +v all the regulars and then if there is noise we can set the channel +m and +v the few extra non-regulars that aren't causing trouble.
< petertodd>
spudowiar: I've got a big project I'm doing in rust right now
< jcorgan>
petertodd: worst case you can always rely on bridgette's money
< spudowiar>
Someone asked me if it worked on Windows so I recorded that for them :)
< petertodd>
jcorgan: heh
< spudowiar>
Wait... petertodd liked that video? When...?
< spudowiar>
Oh wait, late Twitter notifications
< petertodd>
spudowiar: just now :)
< spudowiar>
petertodd: The firmware in the Pong video was written in Rust
< petertodd>
spudowiar: nice! I'm rather excited for rust on embedded; I used to do a bunch of asm and C uC stuff, and it always kinda sucked
< BlueMatt>
re: splitting qt process: can we use our own serialization instead of capnproto, ryanofsky?
< ryanofsky>
BlueMatt: yes we can, i'm working on splitting up the change into two prs, one adding ipc interface and another adding capnproto glue to show how separation works
<@wumpus>
the gui has more pressing problems though: too much happens synchronous with the core in the GUI loop <- in any case, this needs to be addressed first, or as part of the move to a seprate process
<@wumpus>
the GUI thread should never block on a IPC request
<@wumpus>
(currently it blocks on calls to core, or while taking the cs_main lock, resulting in lack of user response for significant time)
< spudowiar>
sipa: If I'm parsing a scriptPubKey using Solver, I get a CPubKey. How to get to a CKeyMetadata from there? (to get hdMasterKeyID)
< gmaxwell>
wumpus: Can you explain shimming in the IPC address those blocks?-- if none of the dataflow is changed.
< ryanofsky>
i'm not changing gui locking / blocking in my pr. if there are cases where gui is freezing up, the only way to address those is individually
<@wumpus>
gmaxwell: shimming?
< ryanofsky>
the only impact my will have on gui responsiveness is there are a lot of ipc calls being called in a tight loop where they might be noticablely slower than current function calls
<@wumpus>
ryanofsky: that's kind of my problem with it; it may compound thecurrent issues
< ryanofsky>
or ipc calls that have to serialize / deserialize a large amount of data
< gmaxwell>
my read on ryanofsky's patch is that it just inserts IPC dispatch at these communication points. The software would still block no less as a result. By shimming I mean replacing function() with ipc->function->ipc.
<@wumpus>
gmaxwell: that's why I say that should be addressed first
< ryanofsky>
wumpus, yes, we will have to see what performance will be
<@wumpus>
gmaxwell: one everything the GUI does is asynchronous, making it work over IPC is also a lot easier
< ryanofsky>
but i do think any performance problems that are uncovered can be dealt with
<@wumpus>
gmaxwell: that was my original plan for the GUI but I never had time to finish it :(
< gmaxwell>
wumpus: ah!
< ryanofsky>
it seems to me a lot easier to fix performance problems that come up than "make everything asyncrhonous"
<@wumpus>
easier, but not better
< gmaxwell>
(I was thinking you thought the ipc changes did this already and thought I was confused).
<@wumpus>
no, ideally IPC chnanges would do that
<@wumpus>
but these don't, no
< gmaxwell>
ryanofsky: is it your thought that eventually the GUI could be started and stopped independantly of the backend?
<@wumpus>
making everything asynchronous would improve the user experience a lot
<@wumpus>
as well as avoid silly things like the gui not coming up because it's blocked on one value that it needs to get under a cs_main
< ryanofsky>
gmaxwell, supporting that would be pretty easy, but i don't know what the use-cases would be
<@wumpus>
ryanofsky: the typical windows service idea: have a GUI to manage it, that runs only part of the time
< gmaxwell>
If not then I really do not understand the purpose of IPC seperating it at all. Making it (largely)-async is an independant issue. The kind of approach here is tightly coupled, so it couldn't really support a GUI in a seperate codebase, multiple GUI projects, or concurrent multiple GUIs.
< gmaxwell>
The advantage of being able to start and stop is so the GUI doesn't run when it's not needed.
< spudowiar>
Oh, wait, I can call CWallet::LoadKeyMetadata!
<@wumpus>
well the cap'n'proto thing could theoretically work with a separate process
<@wumpus>
codebase*
<@wumpus>
they only have to share the interface definition
< ryanofsky>
gmaxwell, i think the approach can support all of those things. but my initial plan for the pr is just to change the infrastructure without exposing new features
< gmaxwell>
wumpus: yes but without a designed and clean interface you can't reasonably support that.
<@wumpus>
gmaxwell: maybe not in the first version
< gmaxwell>
And you get the ZMQ like "everything has to run exactly the same version of ZMQ" issue.
<@wumpus>
gmaxwell: but I think you want too much at the same time
< ryanofsky>
gmaxwell, my pr defines an interface
< gmaxwell>
ISTM if the goal is those things, what is needed is a RPC. This just seems like a 170 degree turn from existing plans on seperating the wallet as a SPV client with special messages for mempool and whatnot.
<@wumpus>
baby steps sometimes make sense so that there is at least progress, the interface he defines may not be super clean yet but it's at least a start
< gmaxwell>
and replacing it with something that we'll never be able to precisely define or secure.
<@wumpus>
anyhow I don't think we'll ever make progress here this way, we just all want too different things
< sipa>
spudowiar: no
< ryanofsky>
how is the interface not precisely designed & secure?
< ryanofsky>
i mean defined not designed
< sipa>
spudowiar: that method is for loading metadata into the wallet
< spudowiar>
sipa: Yeah, I just saw that
< gmaxwell>
ryanofsky: where is a wire specification for captin proto that can be independantly implemented?
< ryanofsky>
oh this is just an objection to capnproto?
<@wumpus>
ryanofsky: even then, you're not currently opening it up, this first experimental version in the PR is internal only
< ryanofsky>
if capnproto is a real problem, a different protocol could be dropped in instead
< gmaxwell>
ryanofsky: yea, a lot of my concern is the approach of taking an internal boundary and then wrapping it in captinproto (which I have specific concerns about which might be outdated).
<@wumpus>
once you have a better idea of what you need you can improve the protocol, then eventually open it up to other applications maybe
< ryanofsky>
most of the work i'm doing here is in making qt not call wallet/node functions directly
< ryanofsky>
if you have suggestions for another wire format to use instead of capnproto, that'd be helpful
< gmaxwell>
ryanofsky: that stuff all sounds great to me independant of the rest.
<@wumpus>
what I like about cap'n'proto is that the interface is cleanly defined in the .capnp file, other software could in principle use that interface definition and just communicate with it
< ryanofsky>
there is a lot i like about capnproto, but so far it actually requires a lot more boilerplate than i was expected, and i'm not at all wedded to it
< gmaxwell>
wumpus: yes so long as they also take all of captin proto, of the same vintage.
<@wumpus>
gmaxwell: yes, they have to use the same IPC library
<@wumpus>
gmaxwell: you'd want an independent implementation of the protocol?
<@wumpus>
I vaguely remember the wire format for capnp is documented, but can't find it quickly
< gmaxwell>
that comes back to IPC vs RPC. I believe the wallet seperation should be a RPC. I don't really have any opinion in GUI IPC seperation, beyond being able to independantly start and stop it, I don't see the value-- but I acknoweldge and respect that people who know more about the GUI parts may see value in it that I don't.
< sipa>
well it would be nice to just have bitcoind running, and occasionally connect your qt frontend to it
< gmaxwell>
If it truly just is an IPC then I don't care much about requring the same versions or an interface that wasn't really designed and documented and supported as a public interface.
< gmaxwell>
sipa: agreed.
< ryanofsky>
i don't really see a significant difference between ipc / rpc here. either way you have bytes going across a socket. the only difference is whether you bind the socket to a port or not
< sipa>
ryanofsky: i think what gmaxwell means is whether it's a well defined interface
< sipa>
will the gui and server always be the same version
< sipa>
or do they need to interact across different versions
<@wumpus>
at first: no
< ryanofsky>
sipa, that's up to us
< gmaxwell>
Well defined and stable, right.
< sipa>
ryanofsky: and i think gmaxwell means that for the Qt split, no well defined and stable interface is needed
<@wumpus>
there's no reason that couldn't be added, but it doesn't seem required for doing this in the first place
< sipa>
but for the wallet separation there is
< gmaxwell>
Also how much do you trust the code you're talking to. With captin proto, if the far end does something unexpected (much less malicious) access to the objects long after the IPC completes will throw exceptions.
< ryanofsky>
capnproto supports extending interfaces in backwards compatible ways, that's the reason for all the field numbers & interface method numbers in the schema file
<@wumpus>
ryanofsky: indeed, they have thought of this
< gmaxwell>
And thats fine for gui/rest since it's all one codebase, from one group.. and of course function calls also do crazy things if you get anything wrong.
<@wumpus>
but yes secure IPC for sandboxed code is difficult, and maybe cap'\n'proto is not the best suited for that
< gmaxwell>
But that isn't what we want for wallet seperation.
< sipa>
i think for wallet separation we should just use p2p
< sipa>
and have the wallet side implement spv
< gmaxwell>
That is what we've always planned, and I think it is good and as a side effect mostly written already.
< sipa>
and designed for
<@wumpus>
looked at chromium: for IPC they use a serialization system (like ours) called pickle, and serialize/deserialize using that in the IPC entry points where needed, no interface compiler
< gmaxwell>
thats totally not confusing with python. :P
< ryanofsky>
wumpus, chromium is in the middle of transitioning between ipc systems
<@wumpus>
communication (on unix) is done over a SOCK_DGRAM or SOCK_SEQPACKET socketpair, they support some advanced things like sending over file descriptors