< ossifrage>
FYI, the usage for 'dumpwallet' really should say that the filename is an output file not an input file. I thought it was an 'input file' and you can imagine what happened...
< wumpus>
luke-jr: how would you propose to do that? allow, say, either an argument amount=1.23 or an argument amount_int=123000000?
< luke-jr>
wumpus: I would propose that whenever named params are used, amounts are always satoshis.
< sipa>
i suggest we at some point introduce a global amount_unit, which modifies the units all amounts are accepted and shown in
< sipa>
which can be done at any time, in a backward compatible way
< wumpus>
sipa: I had a pull open for that at some point
< sipa>
ah!
< luke-jr>
there should be no existing code using named params right now, so there's no need to be "compatible" with fractional numbers
< wumpus>
there were even four options - satoshis as number, satoshis as string, BTC as number, BTC as string
< wumpus>
it could be set using a configuration option, everything using AMountFrom* and *ToAmount would use this
< sipa>
wumpus: well we already support both strings and numbers everywhere, no?
< wumpus>
hardly any interest, though, so eventually rotted and closed. I think despite being a favourite discussion topic for developers, very people are actually concerned about these units on the RPC interface
< sipa>
agre
< wumpus>
arguably interoperability is best by just allowing one representation
< wumpus>
and yes, strings are allowed as inputs
< wumpus>
it would apply to both inputs and outputs though
< wumpus>
in any case I think this is abit of a non-issue
< luke-jr>
so NACK for 0.14? :x
< wumpus>
heh :)
< sipa>
i think it's way too late to change that for 0.14, and given that we already support strings as amount... i'm not sure it's necessary at all
< wumpus>
agree
< luke-jr>
sipa: only reason to push for it in 0.14 is to avoid having to ever support BTC amounts in named-params mode for backwards compat
< wumpus>
let's please not conflate named-param with any other interface change
< sipa>
luke-jr: well we'll always have to support it in non-named-params anyway
< luke-jr>
wumpus: ok, then my idea is controversial and not worth spending more time on
< wumpus>
I mean imagine it from the user point of view, you decide to change your RPC binding to use named arguments, and suddenly you have to use different units!
< arubi>
I guess the 0.8.6 binary doesn't have -regtest (other nodes refusing connection and the switch is missing from -help), 0e4b31755 has it and says 0.8.2 beta in the readme, I guess it didn't become part of the binary until later :)
< luke-jr>
arubi: regtest isn't a network.. you shouldn't ever try to connect it to other nodes (besides in a test environment)
< arubi>
luke-jr, oh I'm well aware. these are all local, running a local regtest network
< luke-jr>
I wouldn't be surprised if it was incompatible with different versions
< arubi>
0.9.5+ worked, thought about trying the older versions
< arubi>
oh I guess only 0.10.0+ worked, I didn't try 0.9.5 yet.
< arubi>
'connect() to 127.3.0.2:18444 failed after select(): Connection refused' :(, wonder if it's just issues in my config..
< arubi>
oh lol
< arubi>
yea, gotta star the daemons first..
< arubi>
anyway, for completeness, I can get 0.9.0 to connect, but not 0.8.6. I guess I'm missing some milestone where regtest was actually released in 0.9.0 :)
< BlueMatt>
sipa: yo
< sipa>
yo
< BlueMatt>
sipa: i think you're missing my point - the precalculations will be done in net/wallet, not that they need the data
< sipa>
that feels wrong
< BlueMatt>
because you want the data available during signing (it doesnt matter much, but its nice), and also in orphans
< sipa>
why do you need signature data for orphans?
< sipa>
you can't verify it
< BlueMatt>
so doing it transparently in the tx object is the cleanest way by far
< sipa>
i strongly disagree
< BlueMatt>
so that its available when you reconstruct the block
< sipa>
this is 1 step forward and a dozen backward
< BlueMatt>
the whole point of the patch is to have the data precalculated when the txn are floating around from various sources (orphan, mempool, recent rejects, etc)
< BlueMatt>
so that when they go into the block its great
< BlueMatt>
and its a serious win
< sipa>
there should be a way to keep it restricted to validation...
< sipa>
we're not putting wallet specific data in CTransaction either
< BlueMatt>
if you insist on CTransaction being a "dumb storage" object, then sdaftuar had a previous version that made CTransactionRef a "transaction and extra data" reference object
< BlueMatt>
its transaction-specific data...you need the sighashes when signing, too
< BlueMatt>
so the wallet does care about this data, technically
< sipa>
i mean we're not putting vfSpent or hashBlock in CTransaction
< BlueMatt>
and I would heasitate to call any hashes of the tx "validation-specific"
< BlueMatt>
sure, because those require otuside data
< sipa>
they're part of the consensus rules, and up
< BlueMatt>
this is all data which is entirely self-contained
< sipa>
not part of serialization
< BlueMatt>
nor is hash, would you like to not cache that?
< sipa>
well i don't like the hash there either
< BlueMatt>
where the hell else would you put it?
< BlueMatt>
its the "in-memory serialization"
< BlueMatt>
sure its duplicative, but its hella useful
< BlueMatt>
and so is witness hashes
< BlueMatt>
at some point this is, by far, the cleanest way to get a massive win, so I really disagree that it shouldn't be done
< BlueMatt>
if you insist on a "data storage class" we can do a wrapper around CTransaction with extra caches (like sdaftuar's original with CTransactionRef becomming a class)
< sipa>
i understand it may be needed now for a performance improvement
< BlueMatt>
this would also pull shared_ptr out of primitives/
< sipa>
but you seem to be arguing that it's overall an improvement and should be the goal
< cfields>
gmaxwell: heh, i've looked at those a bunch for this purpose, but never seemed to come up with anything helpful
< cfields>
it sure _seems_ like it could help, though
< BlueMatt>
gmaxwell: if we were to turn CTransactionRef (or equivalent) into a primitive+cache object, we dont need to take extra overhead, though, I believe
< BlueMatt>
gmaxwell: it can be class CTransactionAndCachedValidationInfo { CTransaction tx; ValidationInfoCache cache; }; typedef CTransactionRef std::shared_ptr<CTransactionAndCachedValidationInfo>;
< BlueMatt>
it gets yucky when you ask what a block contains, though
< BlueMatt>
you end up with like templating CBlock to let it work with either type of transaction object.....
< BlueMatt>
(which is arguably not a layer violation and really doesnt blow up too much, since you pretty much always want the additional validation info in it - only exception is reading block from disk for sending to peer, which could be done via some other function)
< BlueMatt>
but is so damn ugly that I'd heasitate to ever recommend it