< petertodd>
luke-jr: but that's unrelated to the opt-in rbf detection; I agree that in general we want to display doublespends
< luke-jr>
petertodd: well, I mean if we're displaying "replacable" as an indication
< petertodd>
luke-jr: right, but I'm only talking about incoming txs
< GitHub150>
[bitcoin] theuni opened pull request #8067: travis: use slim generic image, and some fixups (master...travis-generic-image) https://github.com/bitcoin/bitcoin/pull/8067
< BlueMatt>
so I was just going back and checking, and the new varint stuff doesnt help anywhere that matters
< BlueMatt>
its an extra few kb in getblocktxn, but thats not really critical-path since that is still one-per-block-per-node
< BlueMatt>
its only like 10 or 20 bytes in cmpctblock, which is the all-important potentially-multiple-times-per-block
< BlueMatt>
gmaxwell: since you're the one with no b/w at home, how much would you care if there is an extra few kb (if even that) in getblocktxn?
< BlueMatt>
somehow I had thought it was a savings in cmpctblock
< sipa>
i'm fine with sticking to the existing varint scheme
< luke-jr>
"one" <.<
< luke-jr>
BlueMatt: How is blocktxn not part of the critical path?
< BlueMatt>
luke-jr: I said getblocktxn
< BlueMatt>
luke-jr: meh, do you care about an extra few kb once per block?
< luke-jr>
right, but that's before blocktxn
< BlueMatt>
indeed?
< luke-jr>
I don't know if it matters; it may not.
< BlueMatt>
it'll be an extra packet or two
< BlueMatt>
and it doesnt effect udp stuff
< jonasschnelli>
luke-jr, petertodd: would you silently ignore (list as normal) incoming RBF txes?
< luke-jr>
jonasschnelli: sure. they are no different in any practical sense than any other incoming txs
< jonasschnelli>
except that they can be replaced.. :)
< luke-jr>
all unconfirmed txs can be replaced.
< jonasschnelli>
Sure. But they don't signal it explicit.
< luke-jr>
irrelevant.
< jonasschnelli>
So why do we have RBF then?
< luke-jr>
because politics
< luke-jr>
if you mean BIP 125
< luke-jr>
RBF in general is just common sense really
< luke-jr>
double spending was always easy for the criminal uses. RBF makes it practical for legitimate users too.
< luke-jr>
BIP 125 is necessary to get around anti-RBF trolls.
< jonasschnelli>
I don't care about politics and trolls. All I care is about a mempool rule that was written down in a Bip (125) and if a tx matches that bip rule (nSequence), it should be visible somewhere (can also be in the tx details once you doubleclick a tx).
< luke-jr>
I don't care if you have the unenforcable nSequence stuff in the tx debug info window you get by double clicking it. I just don't think it should be presented to end users and relevant.
< luke-jr>
mempool policy is not a rule, and is a per-node decision
< jonasschnelli>
I kinda agree. 0-conf should be listed a 0-conf... maybe a slightly different icon (but same color attributes)?
< luke-jr>
there are plenty of nodes and even some miners who do RBF ignoring BIP 125 signalling, and that's not a problem.
< luke-jr>
there's no such thing as 0-conf, it's unconfirmed ;)
< luke-jr>
unconfirmed should of course be presented differently from confirmed, but AFAIK we already do that
< luke-jr>
jonasschnelli: just encryption? or auth too?
< jonasschnelli>
Number assignment for "Authentication" is not required at this point.
< jonasschnelli>
(needs further discussion)
< luke-jr>
k, 151 for encryption
< jonasschnelli>
Super. Thanks!
< luke-jr>
I guess split the PR up so 151 can be merged alone..?
< jonasschnelli>
Yes. I'll remove the auth BIP from the PR and update the enc bip
< gmaxwell>
BlueMatt: I'm sad to continue propagating the old varints in the protocol into more places. They're gratitiously inefficient for no reason... but I don't think the difference between the two really makes a meaningful bandwidth difference in this case.
< gmaxwell>
(the differential encoding otoh makes a huge difference)
< sipa>
gmaxwell: git btw uses little-endian varints, we use big endian ones
< gmaxwell>
BlueMatt: I believe use of the bitcoin one will never cause a increase in packet count, even in the worst case with a 2MB sw block.
< BlueMatt>
gmaxwell: it could with cmpctblock :p
< gmaxwell>
yes, there it could. but not in getblocktxn
< gmaxwell>
consider to get the 3 byte CompactSize, you'd need to have deltas of 254 or more. Well if you have 4000 btc, you can only have 15 such deltas.
< gmaxwell>
s/btc/txn/
< BlueMatt>
those extra few bytes...you never know :p
< gmaxwell>
Well in any case if we cared the fact that small ranges take a whole byte is the real problem. :)
< gmaxwell>
but I wasn't going to suggest you include a range coder.
< gmaxwell>
Though someday when we do a compactblk v2 that uses set reconcil. we'd need a range coder to efficiently encode the transaction ordering...
< gmaxwell>
BlueMatt: in any case, make whatever change you need to make so that I never again hear someone arguing that it should use "UTF-8".
< BlueMatt>
..........
< BlueMatt>
"I wasn't going to suggest"....you keep saying this...........
< BlueMatt>
i mean i dont really give a shit about people making statements that are highly disconnected from reality...just that after having implemented it, I'm not sure its worth the protocol-complexity
< gmaxwell>
BlueMatt: yea, yank it out. I agree.
< gmaxwell>
https://people.xiph.org/~greg/temp/perm.tar.gz permutation encoder that I started writing using the range coder from the daala video codec (which is a fast and efficient multiply free range coder with a nice API), which gets within about 0.5% of information thoretic efficiency. (e.g. it takes 1074 bytes to encode a permutation for 1000 elements).
< gmaxwell>
BlueMatt: the spec should say what happens if you send non-canonical CompactSize encodings.
< BlueMatt>
gmaxwell: i will point out that undefined behavior may eat cats, including non-canonical CompactSize :)
< gmaxwell>
jonasschnelli: sure the rpc and such shows the BIP125 replacability status. My only concern was that we not do something like printing a warning.
< gmaxwell>
jonasschnelli: because a warning would be material misinformation (as it would imply that transactions without the warning were more safe)
< jonasschnelli>
Yes. I agree. I think we should use a slighly different transaction icon (which should not imply a warning)
< gmaxwell>
jonasschnelli: though there are a dozen other characteristics we don't show anywhere in the UI that are often more interesting... we don't show when a transaction uses nlocktime.. we don't show the sighash flasgs of its signatures (for the latter, not even in the RPC).
< gmaxwell>
So I dunno why there would be an indicator icoin for 125 replacablity when there isn't one for nlocktime.
< jonasschnelli>
Yes. We should be careful with what we add to the gui level. Maybe I was to focused on bip125 and thought its important to see it "everywhere". But right,... its not.
< gmaxwell>
oh thats no good. The RPC gets that right.
< gmaxwell>
jonasschnelli: hm. so a downside of that X if it's only mempool conflicted is that it sort of suggests the conflict can't actually go through.
< jonasschnelli>
gmaxwell: the "X" then would show that this transaction is replaced by another one.
< jonasschnelli>
gmaxwell: But maybe there are better ways to deal with that.
< jonasschnelli>
And yes. RPC does handle this, GUI currently not.
< gmaxwell>
which can lead to funds loss, e.g. say I pay Alice and then do something stupid with my wallet and pay Bob. The payment to bob is conflicted with the alice transaction in my own mempool. The Bob payment has an X. Bob is whining that I haven't paid him, so then I go to pay him again, since that txn is "bad" (it has an X). Then both bob payments go through.
< GitHub116>
bitcoin/master 382c871 Pieter Wuille: Use SipHash-2-4 for CCoinsCache index...
< GitHub116>
bitcoin/master 8cc9cfe Pieter Wuille: Switch CTxMempool::mapTx to use a hash index for txids
< jonasschnelli>
gmaxwell: Hmm... how could you create a conflict with the GUI?
< gmaxwell>
jonasschnelli: you can do it by loading two copies of the same wallet at once.
< jonasschnelli>
IMO it would only be possible with the rawtx* API
< gmaxwell>
or using raw txn.
< gmaxwell>
or by loading a wallet backup.
< gmaxwell>
Or by exporting a private key and using it in something else.
< jonasschnelli>
Same problem would be present on RPC listtransaction I guess.
< gmaxwell>
Obviously we try hard to make this difficult to do, but I've seen users do it unintentionally.
< jonasschnelli>
I don't see a easy solution to this.
< gmaxwell>
listtransactions though still will show you the confirmcount of 0. The additional information hopefully would reduce misunderstandings.
< gmaxwell>
I would suggest a ! instead of an X.
< jonasschnelli>
But I see there is a big value in attributing mempool conflicts (with RBF now).
< jonasschnelli>
Yes. Something different then a "X"
< gmaxwell>
"!" suggests there is something going wrong here without as much implication that it is dead.
< gmaxwell>
or something else. Traffic cone. :)
< gmaxwell>
(for a transaction at -6 confirms ... then I would say an X is okay! :) )
< jonasschnelli>
But I guess the "!" transaction would never disappear.
< jonasschnelli>
Once the replaced transaction got mined, the original-replaced transaction would still be in the wallet.
< jonasschnelli>
This might lead to user confusion.
< jonasschnelli>
A right-click-context menu "hide transaction" for some transactions would be good IMO
< gmaxwell>
sure, so long as there is a way to 'show hidden transactions'
< jonasschnelli>
Yes... would be required. Maybe over the options.
< gmaxwell>
A good general UI design principle is that it should never be possible to do something that can't be undone... unless it's at least behind a confirmation.
< jonasschnelli>
Yes. Especially if its a bookkeeping transaction list.
< gmaxwell>
and I think for this having a show/hide and a show hidden in the options menu.. would be okay. Perhaps even an automatically hide conflicted txn with -6 or more anti-confirmations. :)
< jonasschnelli>
Good point!
< jonasschnelli>
Also autohide "!" (mempool conflicts) once the replacement has been mined.
< GitHub126>
bitcoin/master fe80102 Matthew English: changing "(tests are) automatically run" to correspond to the earlier instance of "run automatically (on the build server)"
< GitHub126>
bitcoin/master 457b9df Wladimir J. van der Laan: Merge #8031: improvement to readability...