< GitHub68>
[bitcoin] pstratem opened pull request #8142: Improve CWallet API with new GetAccountPubkey function. (master...2016-06-02-cwallet-getaccountpubkey) https://github.com/bitcoin/bitcoin/pull/8142
< GitHub35>
[bitcoin] laanwj closed pull request #7995: main: Make version bits GUI warning clearer to translators (master...2016_05_minor_message_change) https://github.com/bitcoin/bitcoin/pull/7995
< sipa>
\o/ 4 pages of pull requests
< wumpus>
:o
< wumpus>
luke-jr: I disagree with #8132, we should be adding backwards compatibility code for pulls that were never merged. Also this creates a downward spiral, making it harder and harder to merge because more code is added to be compatible with older versions of the same pull request
< gmaxwell>
I demand backwards compatiblity code for functionality I had in a dream.
< wumpus>
definitely, that should be the next step
< wumpus>
although it's a bit disconcerting that you dream about bitcoind functionality :)
< sipa>
at least we should be backward compatible with the future features we envision!
< sipa>
wait...
< gmaxwell>
am I the only person here that dreams about Bitcoin?
< Chris_Stewart_5>
Thanks -- this is only a hardfork to testnet correct? I was tryign to follow the dev meeting yesterday
< sipa>
jl2012: feel Chris_Stewart_5 indeed
< sipa>
uh
< sipa>
jl2012: feel like updating bip141 for the program size extension?
< sipa>
Chris_Stewart_5: indeed
< Chris_Stewart_5>
sipa: I have to say that is one of the strangest comments i've been tagged on in irc :-)
< sipa>
you clearly need to spend more time on irc then
< sipa>
:)
< instagibbs>
so, will this relaxation only effect v1+?
< instagibbs>
v0 explicitly restricts sizes to 20 and 32
< sipa>
instagibbs: unnecessary complication to do that
< instagibbs>
"do that" meaning to not do it for all versions?
< sipa>
i mean: adding code to make it only affect v1+ would be unnecessarily hard
< sipa>
if you mean whether a naive relaxation will only affect v1+? no
< instagibbs>
yep thanks
< instagibbs>
hm, so the bip is slightly misleading then, if v1-16 was actually size restricted for the witness to 32 bytes max
< sipa>
A scriptPubKey (or redeemScript as defined in BIP16/P2SH) that consists of a 1-byte push opcode (for 0 to 16) followed by a data push between 2 and 32 bytes gets a new special meaning. The value of the first push is called the "version byte". The following byte vector pushed is called the "witness program".
< instagibbs>
I know that, I'm reading later in the text, but I suppose it's actually no contradictory
< instagibbs>
"If the version byte is 1 to 16, no further interpretation of the witness program or witness happens, and there is no size restriction for the witness."
< instagibbs>
perhaps "no further size restriction"
< instagibbs>
oh, witness, not program?
< instagibbs>
not a big deal either way
< luke-jr>
wumpus: gmaxwell: sipa: that one was merged in Knots, and therefore there are wallets in the wild using it.
< gmaxwell>
how did my dreams end up in knots?
< wumpus>
I've had dreams about bitcoin sometimes, though usually about things going wrong
< wumpus>
luke-jr: I understand, but this is quite awkward
< btcdrak>
knots is its own thing...
< wumpus>
I just don't like merging the first version of something then already having to handle an older version of it; it's not like the wallet isn't enough of a mess as it is
< wumpus>
so my question is, doesn't this backwards compatible logic belong in knots but not in bitcoin core?
< luke-jr>
wumpus: depends on how badly it breaks Core to omit it
< wumpus>
well I agree, though there's a limit up to what core will support other people's wallet changes
< wumpus>
it shouldn't crash at least, agred
< wumpus>
(or otherwise lose private keys)
< luke-jr>
the only other possibility I see is it destroying the info, so crash/error seems the least problematic result
< luke-jr>
whatever it is, downgrading would have the same effect, so probably should figure out what it is
< wumpus>
what was the use case why you wanted to merge this so badly before it was finished?
< wumpus>
this is more like sidegrading than downgrading though
< wumpus>
downgrading to a version that doesn't support the functionality at all works
< luke-jr>
there wasn't any reason at the time to see that it would be changed significantly. it had stagnated for months IIRC
< wumpus>
in any case you made it more complex to merge because there's even more scenarios to consider
< luke-jr>
it seems that is the result, yes
< wumpus>
I do think we want the functionality, especially with deterministic wallets, we want to be able to detect if there's e.g. any imported keys
< luke-jr>
unfortunately, foresight is not as good as hindsight; had I known it'd complicate things, I'd have held off - as you say, there was no great urgency
< wumpus>
well it makes clear how careful we need to be with wallet format changes
< wumpus>
if any little change needs to be supported forever, even if it never made it into an release
< luke-jr>
I wonder if we should change the key storage to be like wtx, which uses a key/value map
< wumpus>
that was kind of my reason to hold off on it a bit in the first place, I wanted to be sure not to introduce a problem like this where we'd need a second version
< wumpus>
going from 8 to 32 bits seemed to be a good idea for future extensibility in that regard, although possilbly it's not needed I don't know...
< wumpus>
I suppose we can always make a version 2 and add this migration code *IF* we need 32 bits
< gmaxwell>
"now we can start packing data from the keys inthe version to save space!"
< wumpus>
instead of doing an upgrade for something we're not sure we'll eer need
< luke-jr>
hmm
< wumpus>
ah I see jonasschnelli already proposes that
< * luke-jr>
email down so hasn't caught up on anything yet today :<
< sipa>
i moved ctaes to bitcoin-core
< wumpus>
gmaxwell: hah, I imagine that's what it was like in the 80's where every bit counted
< jonasschnelli>
sipa: ack: +1
< wumpus>
sipa: nice
< wumpus>
8-bit bitfields even sounds very retro, maybe the bitcoind port to MSX can make progress now :)
< jonasschnelli>
:-)
< sipa>
we can stop treating the wallet encrypted keys as padded cbc, and get 12 of storage :p
< jonasschnelli>
Yes. It was my "limited space" that made me use 8bit in the first place. We should really use 32bits for a such thing.
< jonasschnelli>
BTW: is there a reason for not having a function to decrypt the wallet?
< jonasschnelli>
I mean permanently decrypt.
< sipa>
jonasschnelli: yes
< wumpus>
jonasschnelli: just trying to avoid this awkward upgrade scenario
< wumpus>
there should be no need to do that ever
< jonasschnelli>
There is no upgrade scenario right now because we haven't merged anything regarding key-metadata
< sipa>
jonasschnelli: if we'd design it from scratch, i think wallets would always be encrypted (though perhaps with an option to have an empty key)
< jonasschnelli>
But I think a 32bit bitmap for key metadata could make sense for a wallet upgrade.
< jonasschnelli>
Can be use to determin where the key came from, if it's HD generaded, etc.
< jonasschnelli>
sipa: Good point.
< jonasschnelli>
Should I take a second attempt use use LogDB for the wallet database?
< jonasschnelli>
So we could use a simple subset for bitcoin-core (not a subtree, more like 2-3 files like ctaes)
< jonasschnelli>
I could even add callbacks for the hashing to avoid duplicated sha256 implementation.
< wumpus>
I agree with sipa. Optionally allowing an empty key would be ok, it would still avoid some file system leaks
< jonasschnelli>
Right. I also think we should support *full*-wallet-encryption.
< jonasschnelli>
(require unlock of level 1 when staring Bitcoin-Qt/bitcoind)
< jonasschnelli>
(require unlock for level 2 when signing stuff)
< luke-jr>
(IMO wallet encryption is mostly a PR stunt from 2011, and we should focus on hardware wallet support.)
< jonasschnelli>
luke-jr: +1.
< gmaxwell>
I worry multiple passwords means the user will forget the signing one and not realize there are two.
< jonasschnelli>
I once started to specify the "detached signing".
< jonasschnelli>
Sadly there is no standard API for hardware wallets...
< gmaxwell>
we really don't want to make the data loss worse from wallet encryption, I'd rather have it gone than that.
< jonasschnelli>
gmaxwell: maybe the data loss is also because we don't offer a clear recovery-process during encryption.
< luke-jr>
gmaxwell: could we have them be the same, and only cache the decryption key (not the passphrase) at runtime?
< jonasschnelli>
Like allowing the user to write somthing down or print out a "backup" key or something.
< gmaxwell>
I suggested an idea a while back that we just define an interface where we fork a process that speaks a bitcoin-core specific protocol to bitcoin... then speaks whatever the wallet needs to speak, and can open up UIs and whatnot.
< gmaxwell>
e.g. signhardware=bobpocketwallet-qt.exe
< wumpus>
I'm not sure about full wallet encryption, you can always store the wallet on an encrypted volume that's likely safer
< wumpus>
(you can store your other secret files there too.)
< jonasschnelli>
I was thinking after more torwards TCP/IP httpd interface
< wumpus>
I doubt the bitcoin wallet metadata is the only metadata you'd want to hide
< gmaxwell>
wumpus: the model I like is where you use the signing key to derrive an access key (e.g. H(KDF(passphrase)) = viewkey) and then you simply save the viewkey on disk in a seperate file.
< wumpus>
yes, support for hardware key storage and signing would be nice
< gmaxwell>
wumpus: then if you backup/restore (e.g. to the 'cloud') you'll need to enter your passphrase once to unlock.
< wumpus>
(or "isolated CPU conclave" if that's what you prefer)
< gmaxwell>
but your backups are still confidential without extra steps.
< jonasschnelli>
Each hdwallet could offer a tiny daemon (httpd) that listens for requested sign processes and build up a UI once a signing-request comes in.
< jl2012>
sipa: what's the decision?
< jonasschnelli>
hdwallet=hardwarewallet
< sipa>
jl2012: 40 bytes
< wumpus>
gmaxwell: yes, that is a good idea.
< gmaxwell>
Yea, on your local meachine metadata privacy is almost totally pointless, your browser cache tells anyone with your computer almost anything you did with the bitcoin wallet.
< gmaxwell>
but for backup it's useful.
< wumpus>
two passwords is too much for most people
< jonasschnelli>
Yes. That's true.
< jl2012>
Ok, will do it tomorrow
< jonasschnelli>
Maybe encrypting the disk is the way to go.
< wumpus>
well the wallet encryption is for local security I suppose. Backups you'd certainly want fully encrypted, I agree
< wumpus>
OTOH that doesn't need to be wallet.dat format
< gmaxwell>
encrypting your disk is a great idea. I've encrypted all my disks since .. uh.. 1998? WD sent me back an RMA drive with someone elses data... often when a drive fails you can't zeroize it first.. sooo.
< gmaxwell>
wumpus: thats true, wrt format.
< luke-jr>
I prefer to encrypt only my sensitive data, so I can un-decrypt it when I step away
< wumpus>
I encrypt my disks too, except for 'junk' partitions like where I store zillions of copies of the blockchain
< jonasschnelli>
heh
< wumpus>
I suppose with a newer CPU with encrypt/decrypt instructions the overhead is so low that you don't really care and just encrypt everything
< wumpus>
(I do always encrypt swap with random key too)
< wumpus>
in any case, for backups encrypting the full thing makes a lot of sense, certainly for cloud backups
< wumpus>
dropbox must have a lot of wallet metadata
< luke-jr>
for backups, definitely want to encrypt the whole thing IMO
< wumpus>
yes
< wumpus>
I just encrypt my entire backups so such functionality in bitcoind wouldn't help me much, but I guess some people would be helped by it
< gmaxwell>
encrypting it is also good for metadata preservation, e.g. evenutally we could have some ftp or webdav or whatever kids uses these days, url in the config you could set to push a new backup every time your metadata changes.
< luke-jr>
well, hopefully that new BIP metadata storage thing works out..
< sipa>
can't we just store the metadata in a dht cloud blockchain, with rainbow tables for security?
< wumpus>
and render the rainbow tables in actual rainbow colors in the GUI
< wumpus>
with dancing unicorns
< wumpus>
nothing protects your wallet better than distributed colorful random cloud technology
< sipa>
copied from a random github repository
< luke-jr>
…
< gmaxwell>
back it up by finding a random pull reqest and then open up a copy against a fork of the origin project with the data added...
< gmaxwell>
non-zero probablity that they merge it.
< sipa>
a certain linux thorvalds quote comes to mind
< cfields_>
MarcoFalke/wumpus: re: #8133, suggestions for how to fixup the import paths? I used a quick hack for the first one, but since we need another, i'd rather avoid adding more hacks on top
< MarcoFalke>
Would also help if there was a comment for the hack, so it is easier to understand when looked at later.
< MarcoFalke>
Did you manage to look at the python issue?
< MarcoFalke>
I can try to play a bit locally when that's fixed
< cfields_>
MarcoFalke: yea, it's the same issue that the "a few ugly hacks" commit fixes, just a different place.
< cfields_>
MarcoFalke: ok, maybe easier if i just apply another ugly hack for now, and you can run cleanup afterwards? :)
< MarcoFalke>
If I can figure out something better ;)
< cfields_>
MarcoFalke: the underlying issue is that the scripts in the read-only srcdir need to import a script from the builddir, which is in an unknown location
< MarcoFalke>
ok
< cfields_>
the hack fix is to assume we're running from builddir, and add the path of the to-import files relative to pwd before doing the actual import
< cfields_>
MarcoFalke: aha, wait. we already have the path set in the makefile. The problem is the .pyc lingering around in srcdir
< cfields_>
(that's why travis passed)
< cfields_>
ok. since we require python3 now, i don't think that's a problem. looks like it should just be a one-time delete.
< MarcoFalke>
cfields_: I am missing the background but what about adding the rm *pyc to `make clean`?
< MarcoFalke>
The same pycs need to be removed in the qa folder, I guess
< MarcoFalke>
(Same error)
< cfields_>
MarcoFalke: 'make clean' now removes __pycache__, which does just that
< cfields_>
MarcoFalke: it's just the old python2 .pyc's that are trouble. More specifically, only when the .pyc exists and the .py is in a different path (builddir/srcdir)
< MarcoFalke>
When I first tried to build out of dir, it told me to do 'make distclean' in the src dir.
< cfields_>
MarcoFalke: you got another error in qa? tests_config.pyc i'm guessing?
< MarcoFalke>
So keeping the code to remove pyc's in `make clean` temporarily would make sense
< MarcoFalke>
jup, test_config
< cfields_>
MarcoFalke: aha, good point!
< cfields_>
MarcoFalke: so adding those 2 files to the distclean would solve it in a more obvious way.
< MarcoFalke>
I guess so
< MarcoFalke>
Also there is pyc's in qa/rpc-tests/test_framework/ like __init__.pyc
< MarcoFalke>
I am assuming they don't hurt?
< cfields_>
MarcoFalke: I believe it's only the ones that don't exist in srcdir (the pre-processed files)
< cfields_>
MarcoFalke: pushed with that change instead. thanks for the reminder about the forced distclean.
< MarcoFalke>
testing...
< cfields_>
MarcoFalke: it might cause the same error for you now, but remember that you're passed the forced distclean. You can do another to simulate.
< MarcoFalke>
distclean works
< cfields_>
ok great. thanks for testing!
< MarcoFalke>
copying rpc-test.py instead of linking would be considred bad practice?
< cfields_>
MarcoFalke: we could, but I was afraid it'd get confusing.
< sipa>
would it help to move roc-test.py to the roc-tests directory?
< * luke-jr>
doing some rather extensive testing with 8133 FWIW