< sipa>
jtimon: since 10195, at first startup, your chainstate database will be upgraded to a new format
< sipa>
this may take a while, but only happens once
< jtimon>
sipa: thank you, and gmaxwell is pointing out that trying to downgrade the chainstate database format would be painful, but there's no good reason to want that besides testing, right?
< sipa>
indeed
< jtimon>
but gmaxwell tried anyway, nice
< jtimon>
and it works
< jtimon>
I guess I shouldn't have started with the youtube video
< jtimon>
I know it's selfish, but my plan was to partially review 10195 after squashed and merged all along
< jnewbery>
sipa: do you particularly want it? I didn't sense there was all that much enthusiasm for it
< spudowiar>
Can I use C++11 std::map::at()?
< sipa>
spudowiar: yes, but i would advise against relying on exceptions
< spudowiar>
Why?
< sipa>
especially in the case of at; you can just use auto it = map.find(key); if (it != map.end()) { ... } else { ... } instead
< spudowiar>
Ah, I'll do that instead then
< spudowiar>
Thanks!
< sipa>
jnewbery: i conceptually like i very much... i think make check should do ~all reasonable checking
< sipa>
but i understand there are concerns that make the choice of what to run where and when hard
< jnewbery>
yeah - I couldn't seem to converge with others on what's a sensible choice of what to run
< jnewbery>
I'll probably pick it up again at some point, but it shouldn't really be in the review priority bucket since there's nothing to review at this point
< sipa>
perhaps something to bring up as a meeting topic
< sipa>
agree with removing it from priority review list
< spudowiar>
Do you have any qualms with executing a command and piping data into it?
< spudowiar>
Also, are there any examples of this in the Bitcoin Core code?
< spudowiar>
Before, I was using popen but now I want to clean up this patch in order to submit it
< spudowiar>
Should I be adding more code using boost? Because I could use boost::process for this
< spudowiar>
I mean, should I be avoiding using boost?
< bitcoin-git>
[bitcoin] luke-jr opened pull request #10512: Rework same-chain from abusing DoS banning, to explicit checks (master...samechain_rework) https://github.com/bitcoin/bitcoin/pull/10512
< spudowiar>
gmaxwell: Is JSON alright for serializing data for hardware wallet support? I think it'll be easier for the external tools than normal Bitcoin serialization
< gmaxwell>
spudowiar: almost certantly not, needing megabytes of ram to buffer such a thing require several extra dollars in parts.
< spudowiar>
No, not on the actual hardware wallet
< spudowiar>
For the vendor specific tools
< gmaxwell>
spudowiar: existing hardware wallets go through serious work to avoid even having to buffer a single transaction, much less a json encoded one.
< gmaxwell>
uh? perhaps but you have to be able to handle the bitcoin seralization in order to compute any hashes over it.
< spudowiar>
No, because most hardware wallets serialize it themselves
< spudowiar>
Although I have a very complete understanding of Trezor and a very limited ones of others
< spudowiar>
Btw, didn't jonasschnelli's hardware wallet used to use JSON :)
< spudowiar>
Anyway, I was using Protocol Buffers in my PoC but I knew I couldn't submit that because you'd probably kill me ;)
< spudowiar>
Basically my patch takes an argument -hardwarewallet=<cmd>
< spudowiar>
When you spend with the wallet, it executes the command, pipes in the transaction (in Protocol Buffers at the moment) and the command returns the serialized transaction
< spudowiar>
Then Bitcoin Core verifies that
< spudowiar>
If there's an error, it returns a non-zero status and the message on stdin is used as the failure message in Bitcoin Core
< * luke-jr>
idly ponders if there's a way to do that such that bitcoind is itself a valid -hardwarewallet
< gmaxwell>
I don't see why you wouldn't use the ordinary serialization plus metadata, _any_ hardware wallet needs to be able to handle the serialization of transactions. Plus how would you proprose to handle things like coinjoins and partially signed multsigs?
< spudowiar>
I don't have bitcoind as one, but I have a script that talks to a bitcoind over RPC
< spudowiar>
gmaxwell: Hardware wallets don't deserialize the transactions, they always accept it in a different format
< spudowiar>
JSON is so much easier because, otherwise, each tool has to deserialize the transaction
< spudowiar>
Partially signed multisig, on a TREZOR, is done totally differently to a P2PKH
< spudowiar>
luke-jr: ln -s bitcoind bitcoind-hardwarewallet and do an argv check :)
< gmaxwell>
spudowiar: of course they do, e.g. to pass them the inputs for value checking you must pass them the input transactions exactly.
< spudowiar>
Oh, yeah. But they don't deserialize the to-be-signed transaction
< sipa>
then how do they compute the sighash?
< spudowiar>
They serialize it from their own format
< spudowiar>
e.g. TREZOR uses Protocol Buffers
< luke-jr>
spudowiar: i was thinking more of using JSON-RPC over stdio
< spudowiar>
That's an interesting idea
< spudowiar>
Because a hardware wallet could ask for transactions when it needs them, etc.
< spudowiar>
Anyway, should I be adding more uses of boost? Was thinking of using boost::process
< spudowiar>
In my PoC I used popen and pclose but that's not very C++-esque
< gmaxwell>
it just seems like a total waste of time and effort to define a whole new seralization which has to be completely compatible and able to encode everything a transaction can encode.
< gmaxwell>
Whats the purpose?
< luke-jr>
gmaxwell: HW wallet vendor provides a plugin for Core
< spudowiar>
But then each script has to deserialize the transaction which seems like a total waste of time ;)
< gmaxwell>
spudowiar: that isn't escape by using a _different_ seralization.
< spudowiar>
Python, for example, has built-in JSON support
< spudowiar>
JSON-RPC over stdio seems like a neat idea though
< spudowiar>
gmaxwell: What about using the format for decoderawtransaction (possibly with a bit more metadata, if needed)
< sipa>
whatever you do, please don't try to represent multisig as multiple addresses :)
< bitcoin-git>
bitcoin/master b9b814a Russell Yanofsky: Avoid CWalletTx copies in GetAddressBalances and GetAddressGroupings
< bitcoin-git>
bitcoin/master 098b01d Pieter Wuille: Merge #10500: Avoid CWalletTx copies in GetAddressBalances and GetAddressGroupings...
< bitcoin-git>
[bitcoin] sipa closed pull request #10500: Avoid CWalletTx copies in GetAddressBalances and GetAddressGroupings (master...pr/wtxcopy) https://github.com/bitcoin/bitcoin/pull/10500
< luke-jr>
hm, 0.14.2 seems to have missed some fixes still :x
< gmaxwell>
spudowiar: but you can't do anything with bitcoin transactions without also having bitcoin transaction ser/des support! and then you have to worry about that your json format cannot losslessly represent a transaction. Decoderawtransaction cannot. E.g. it can't encoding different choices for encoding in varints.
< luke-jr>
gmaxwell: the other end would translate the JSON into some hardware interface; the hardware wallet itself does the serialisation
< luke-jr>
ie, there's a middle-man who has no need to understand ser/des
< spudowiar>
^^
< aj>
gmaxwell: (post-segwit you don't want the serialised input tx, you just want the txid, value and some signing key id, no?)
< sipa>
yes
< luke-jr>
spudowiar: note that using JSON-RPC means bitcoind will call signrawtransaction, and you'll have to deserialise (or pass as-is?)
< spudowiar>
What do you mean? I was thinking of sending the current transaction then the hardware wallet could ask for input transactions (and the script would use JSON RPC to grab them)
< gmaxwell>
aj: not for the inputs, but you still want the whole transaction itself.
< gmaxwell>
aj: I think it would be fairly hard and at least wasteful to define a whole new serialization that is a guarenteed superset of the transaction format. I think spudowiar is thinking that you can just say {pay inputs x,y,z to destination a,b,c} but that doesn't work if the hw wallet isn't the author of the whole transaction.
< spudowiar>
gmaxwell: that is literally what all hardware wallets do right now
< spudowiar>
Even for multisig, they don't accept a serialized transaction
< arubi>
(this is why I was requesting raw sighash support :) )
< aj>
gmaxwell: yeah, i think i agree; i think you just want to send the serialised partially-filled out tx you want to create/sign, and extra info needed to do the signature (txids, tx values, pre-segwit-serialised-input-txes, SIGHASH params, etc)?
< gmaxwell>
spudowiar: that isn't true; ledger takes seralized transactions.