< bitcoin-git>
[bitcoin] fanquake closed pull request #14463: Reduce usage of the platform dependent `unsigned int` type (master...20181011-rbf-nseq) https://github.com/bitcoin/bitcoin/pull/14463
< sipa>
MarcoFalke: it seems that editing a comment indeed moves PRs up the "recently updated" queue
< gwillen>
:-(
< hebasto>
@fanquake: hi, regarding #14597 "our next release will require at least 5.2"; i've read through #13478. Can you point me to related discussion?
< fanquake>
hebasto 13478 is the bulk of the discussion, I can't imagine we'd support < 5.2 when we release 0.18.0, and master currently doesn't support < 5.2 ,see #14078 (unless that's been fixed, but I don't think so).
< hebasto>
@fanquake: thank you. it gets better :)
< fridokus>
Hey. I have a question about the functional test FW. If this is the wrong place please redirect me. In wallet_abandonconflict.py line 39, we get an RPC error because txA is already included in a block. But how can node0 know this, when the block was generated on node1 (line 34) and the blockchains haven't been synced?
< provoostenator>
fridokus: the mempool is synced in line 33
< aj>
provoostenator: but node[0] might not have seen the block at line 34, so why would it think it's confirmed?
< provoostenator>
Right, that explains the first test, but test in line 39 is strange
< provoostenator>
RPC error messages are not always specific enough.
< aj>
okay, in my test, the blocks get synced immediately, without needing sync_blocks()
< provoostenator>
aj: that could also explain it, in that case maybe remove sync_blocks from that test to prevent confusion?
< aj>
or move it up to just after generate() to make it more self documenting?
< provoostenator>
But all that RPC error says is that !pwallet->AbandonTransaction(hash), which could be for other reasons.
< provoostenator>
Wallet::AbandonTransaction checks that the transaction isn't in a block, so that could indeed trigger the failure. But it checks a whole bunch of other things too. So a better test would delay block sync.
< provoostenator>
Or a more useful return type than bool.
< provoostenator>
Actually all other checks are asserts, so it's fine in this case. Though in general I think errors should be checked more precisely.
< aj>
provoostenator: hmm, if i run generate() sync_blocks() node[0].invalidateblock() it still seems to give the "can't abandon" error?
< aj>
provoostenator: if i call abandontransaction before generate() it gives the error too.. something seems wrong?
< aj>
oh, being in the mempool suffices...
< fridokus>
Ok thank you both, it is clearer for me now
< fridokus>
aj: you mean that a tx being in the mempool is enough so that we can't abandon it?
< aj>
fridokus: yeah
< fridokus>
Then we should remove sync_blocks in line 41 to prevent confusion I think.
< bitcoin-git>
[bitcoin] fridokus opened pull request #14638: Tests: Remove sync_blocks() from wallet_abandonconflict.py (master...develop) https://github.com/bitcoin/bitcoin/pull/14638
< promag>
what is the correct convention for rpc arguments? foo_bar, fooBar, foobar, ...?
< promag>
sorry, it's in developer notes..
< bitcoin-git>
[bitcoin] promag opened pull request #14641: RPC: Add min/max confirmation options to fund transaction calls (master...2018-11-fundrawtransaction) https://github.com/bitcoin/bitcoin/pull/14641
< gwillen>
if anybody expects to have opinions on offline signing UI, I have a branch they could try and criticize.
< gwillen>
(a bit tangential probably.)
< gmaxwell>
gwillen: neat. where?
< sipa>
gwillen: unsure, want to give a short update on what you're trying to achieve?
< gwillen>
The goal is to take the PSBT stuff and bring it into the GUI in an interface somewhat like Armory has for offline signing.
< gwillen>
It adds a 'spend watchonly' checkbox to the send dialog, and another dialog for importing and exporting PSBT blobs, signing and broadcasting them.
< gwillen>
It works right now, but all the instruction labels are placeholders and the code is janky.
< gwillen>
And I think I have decided that copying the Armory interface is not the best approach, and in reality users do not need to see base64 blobs during this process (at least, without clicking 'advanced' somewhere.) Better to just show them what the transaction will do.
< gwillen>
So that will be the next revision.
< sipa>
gwillen: so you still plan to change the UI/concept significantly before PRing?
< sipa>
or after
< gwillen>
I think I should change it before PRing, unless you think it would be better to let people give feedback.
< gwillen>
But people are welcome to try out the branch, and I will try to keep it compileable.
< sipa>
cool
< sipa>
as far as the descriptor/importmulti/wallet progress goes, i think most was said yesterday in the meeting
< instagibbs>
ah shoot, wallet meeting
< sipa>
should we go over it again, perhaps in more detail?
< instagibbs>
need to add to calendar
< sipa>
also, provoostenator: ping (since he was excited to hear there was a wallet meeting now)
< jnewbery>
achow101: ping
< achow101>
hi
< gwillen>
instagibbs: remember to put it in iceland time so you don't get smacked by DST
< instagibbs>
gwillen, already do for thu meeting
< instagibbs>
gwillen, can you elaborate on what non-blob UX would look like?
< meshcollider>
provoostenator was in the ping list at the start so he should have already been notified :)
< gwillen>
instagibbs: I have been thinking over the last day or two, the first thing that comes to mind is just to replace the blob with the contents of the current "are you sure" dialog, i.e. "this is a transaction that will spenx XX to YY with fee ZZ, are you sure you want to [create/sign/broadcast/etc.]?"
< meshcollider>
sipa: are you happy to write the IsMine/keypool abstraction and everything yourself or would you like a minion ;)
< instagibbs>
but base64 stuff has to be printed for copying.. right?
< gwillen>
well, I think in most cases people are going to want to use file save/load instead of copy/paste, in fact I can't think of a case where copy/paste makes sense when using multiple machines (as opposed to when testing the PR)
< sipa>
gwillen: also note that #13932 adds an RPC that tells you waht the next steps are for a PSBT (get X to sign, update input Y, broadcast, ...); may be useful for your UI
< achow101>
gwillen merging transactions in what way?
< sipa>
achow101: psbt combiner
< sipa>
gwillen: i assume ^
< gwillen>
achow101: in the combinepsbt sense, yeah -- perhaps armory has that now, I am on an ancient version and I don't use multisig
< achow101>
there's already combinepsbt, but 13932 has joinpsbts to join independent txs
< gwillen>
oh, I see, interesting, thanks
< sipa>
achow101: gwillen is talking about his GUI
< sipa>
meshcollider: so i haven't thought that much about the abstraction, but perhaps we can discuss ideas in PM or so?
< meshcollider>
sipa: sounds good yep :)
< sipa>
so i think these things can all be done in parallel now: (a) add RPCs to sign/update a PSBT with a descriptor/utxoset (b) create an abstraction for "list of addresses" (that encapsulates the current keypool/hd derivation logic) (c) extend the descriptors code to cache pubkeys (so it can be used as a keypool later, even when hardened derivation is used)
< sipa>
any other things people want to discuss?
< jnewbery>
One thing. A little off topic for a core wallet meeting, but I think people here might be interested.
< jnewbery>
Optech are holding our second workshop in a couple of weeks. We're getting a bunch of engineers from wallets/exchanges together and we'll discuss: RBF/CPFP, PSBT, output script descriptors, lightning integration and coin selection.
< jnewbery>
We'll report back on what we learn, but if people here have any specific questions that you'd like us to ask and get feedback on, please let me know.
< jnewbery>
PSBT especially could be useful, since everyone would benefit from wider adoption
< gwillen>
jnewbery: since I am planning on saving/loading PSBT files for creating/signing/broadcasting, it would be good to be on the same page as other wallets on workflow, so that we can be cross-compatible
< gwillen>
I have been advised the the correct PSBT file format is just the raw PSBT bytes, not base64 or any other encoding
< gwillen>
so compatibility should be easy
< gwillen>
I am interested in any feedback on that from other wallets
< jnewbery>
gwillen: sure. We'll be writing up notes and I'll share them with you
< gwillen>
Thanks!
< instagibbs>
achow101, I can write HWI support :)
< gwillen>
achow101: what do you think about the idea of moving the non-wallet PSBT methods into their own file?
< gwillen>
I can't use RPCs directly from the GUI, so I have to refactor them into (1) a general-purpose method that operates on PartiallySignedTransactions and (2) an RPC method that calls it
< gwillen>
and it seems like maybe all the (1)s should go in a file together outside of /rpc/
< sipa>
gwillen: that sounds great
< gwillen>
(sorry for thinking out loud here but it doesn't seem like the meeting has an active topic)
< instagibbs>
Maximal reuse sounds amazing, please do
< gwillen>
sipa: cool, where would you put it?
< gwillen>
(I mean, in the directory structure)
< sipa>
gwillen: script/psbtutils ?
< gwillen>
also this is going to start getting messy with stacked unmerged refactors
< gwillen>
which I guess is why refactoring is discouraged ;-)
< sipa>
i don't think it will interact badly
< gwillen>
it will be slightly annoying, not horrible
< sipa>
when merged, the wallet interfaces/ will need extra methods to access that psbt logic, but that's pretty much just additive, i think - not conflicting
< gwillen>
well it's going to rebase annoyingly against things that are messing with the PSBT RPCs, so #13932 #14588, maybe that's it
< gwillen>
instagibbs: I would probably put 'copy to clipboard' under 'advanced', which will also show the base64
< instagibbs>
oops right
< gwillen>
if you have any thoughts on cases when people would actually use that feature I am all ears, as that would influence how I would design it
< instagibbs>
my dumbery means I'm advanced thanks
< gwillen>
but I realized I never would except for testing
< gwillen>
(the really cool thing would be QR codes, clearly ;-)
< gwillen>
(I am only half joking here)
< sipa>
I don't see why that's a joke
< achow101>
turns out this meeting time isn't so great for me
< gwillen>
sipa: well, I have always been slightly skeptical -- people are suspicious of USB sticks, but a USB camera is a much more complex device than a usb stick
< gwillen>
if your offline machine has a camera built-in then it might be a good approach
< sipa>
gwillen: ah, fair
< sipa>
for the other direction (offline to online) it makes perfect sense though
< gwillen>
(also, the ergonomics of pointing a laptop's built-in camera at another laptop are terrible)
< gwillen>
yeah, that's definitely true
< instagibbs>
ColdCard, for one, only uses files natively from what i understand, so there's at least 1
< gwillen>
do you know, does it use the "obvious" format (raw PSBT bytes in a file by themselves, no header, no base64 or other encoding)?
< achow101>
gwillen: it uses both
< gwillen>
i.e. it will read either raw bytes or base64 bytes?
< achow101>
yeah
< gwillen>
does it just write back the same as it read?
< jnewbery>
One piece of feedback I had from a wallet was that they didn't like PSBT compared to what they're currently using because it's large and doesn't fit in a QR code. I haven't dug into that yet
< achow101>
jnewbery: I think that is unfortunately a byproduct of needing to maintain trustlessness. if you trusted the person making the psbt, then you it wouldn't need to be so large as non-witness utxos need not be the entire previous tx
< gmaxwell>
even without inputs, IIRC the maximum size of a QR code is 4kb..
< gmaxwell>
meaning you're not even remotely close to being able to carry back the signatures in a maximum standard txn with just one QR code.
< sipa>
the QR spec defines a way to split data over multiple QR codes :)
< instagibbs>
qr slideshow, oh boy
< achow101>
meshcollider: can you rebase your importmulti descriptors pr onto 14565 so I can build something on top of it?
< phantomcircuit>
sipa, which iirc nobody implements
< phantomcircuit>
also very large qr codes tend to break things
< jnewbery>
yes, you can't carry the signatures in a maximum standard txn with a QR code, but most of the time people aren't creating maximum sized standard txns
< sturles>
I wonder - would it be time to end the practice of having a fixed absolute minimum relay and mining fee, and allow for some slack e.g. when a node's mempool is below 100k? It could allow for, and encourage, some UXTO cleaning at times of low transaction rates. The dust limit can stay the same as now.
< gwillen>
achow101: this is interesting actually, if you are space constrained you really do just need the parts of the PSBT that the sender doesn't already know, i.e. the sigs
< gwillen>
except there's no spec for determining that
< gwillen>
I dunno if this is useful enough for that to actually exist or not
< luke-jr>
sturles: I don't see a rationale for that; bandwidth and CPU time doesn't get cheaper just because demand is low
< luke-jr>
maybe the absolute minimum can be lowered, but it doesn't seem logical to remove it entirely
< meshcollider>
achow101: sure :)
< sturles>
There should be more bandwidth and CPU time available when demand is slow.
< hebasto>
jonasschnelli: hi! what is the purpose of "#undef slots" in "src/qt/macdockiconhandler.mm" ?
< phantomcircuit>
sturles, iirc the minimum relay stuff is very low already
< gwillen>
hebasto: it looks to me like, at least at one time, some of the macos stuff used "slots" as a name internally somewhere in the headers
< gwillen>
but QT has a #define of "slots" for its own unrelated purpose
< gwillen>
(which is gross because one should not go around #defining lowercase names, but QT something something; there is a config option you can set to make it not do that)