< conman>
yeah I was just randomly picking them off the bottom and looking at them
< conman>
silly me
< conman>
since I know nought of this
< conman>
ok I prioritised the lowest priority one I could find
< gmaxwell>
FWIW, it worked for me (with 0.18rc) -- I picked the bottom segwit txn, prioritised it, and it's now the second transaction in my template.
< conman>
and now it no longer appears in the block template at all
< conman>
let's try again
< gmaxwell>
OH how high is your priority? is it possible that there is an overflow somwhere?
< gmaxwell>
I did: $ ./bitcoin-cli prioritisetransaction 35611cbad11967731aa122136bf90bd63c5224296c45c10a26885c3bb3fff6f6 0 100000
< gmaxwell>
true
< conman>
0.1btc
< conman>
so 10000000
< conman>
again, same thing happened this time around
< conman>
I'll try smaller then
< conman>
but note that size priority worked fine on a regular transaction
< gmaxwell>
I tried 10000000 ... now its the first one.
< conman>
then wtf is going on here
< gmaxwell>
bug in 0.17 that is fixed in 0.18? I just don't recall any such changes.
< conman>
every prioritised one disappears instead of moving up
< conman>
hang on, maybe I'm searching for it wrong in the template
< gmaxwell>
I'm just ./bitcoin-cli getblocktemplate '{"rules": ["segwit"]}' | jq '.transactions | .[] | select(.hash != .txid)' |less and hitting / and pasting in the txid.
< gmaxwell>
and I successfully move something to the top, then undo the priority and it goes back down to near the bottom.
< conman>
I'm not adding rules to getblocktemplate...
< conman>
is that what I'm doing wrong?
< conman>
doesn't explain how it's been generating templates with segwit txns all the time
< conman>
I was just doing getblocktemplate | grep
< conman>
or | less and then / searching
< gmaxwell>
oh man..
< conman>
:(
< * conman>
hides
< gmaxwell>
So I did ask...
< gmaxwell>
in 0.18 this isn't a problem:
< gmaxwell>
$ ./bitcoin-cli getblocktemplate
< gmaxwell>
error code: -8
< gmaxwell>
error message:
< gmaxwell>
getblocktemplate must be called with the segwit rule set (call with {"rules": ["segwit"]})
< conman>
I see
< gmaxwell>
but for backwards compat reasons gbt won't include any segwit txn unless called with rules (and in 0.18 that functionality is disabled)
< conman>
I'm just going to rock back and forth in the corner
< gmaxwell>
I would have asked harder but I didn't remember when the backwards compat was turned off, I kinda assumed 0.17 had it too, so I didn't ask more than once. Sorry.
< conman>
don't be sorry, it's all my fault
< gmaxwell>
this was a known footgun thus the new behavior in 0.18.
< conman>
for some reason I thought the rules were on by default
< gmaxwell>
yea, they aren't just because if you had non-segwit capable mining software you'd start silently mining invalid blocks.
< gmaxwell>
And we'd rather it fail cleanly.
< conman>
ok well that's been a big learning experience
< conman>
for all the wrong reasons
< gmaxwell>
thanks for the confirmation that 0.18's behavior change is a useful de-footgunning. :P
< conman>
of course the pool software doesn't do that so no idea why I didn't think of it for the command line :s
< conman>
I really have to get out of this game, it's bad for my health
< conman>
thanks again
< gmaxwell>
sorry, this interface was bad-- just an annoying consequence of backwards compatiblity. That much isn't your fault. :)
< gmaxwell>
I'm glad it wasn't a weird bug though.
< * conman>
quietly disappears
< gmaxwell>
Next Customer!
< * gmaxwell>
is sad that he has still not learned the zen of tech support.
< gwillen>
gmaxwell: can you help me with this terrible pain I have in all the diodes down my left side
< gmaxwell>
gwillen: have you had them replaced?
< gmaxwell>
When I worked for Juniper I was often amazed at how the really good tech support people managed to solve some feindishly hard problems that no one else could solve by being single mindedly consistent about asking all the dumb questions and carefully validating every claim and piece of information.
< gmaxwell>
And yet, no matter how much I tell myself that I _must_ ask the 'dumb' questions to get compariable results, I still manage to fail.
< wumpus>
gwillen: yes the global coin control object is kind of ugly, and very much different from how the rest of the GUI code works; basically because the original author wrote it like that, and people kind of wanted the functionality so didn't want to hold up the merge for it, it was expected that someone would improve it down the line
< wumpus>
gwillen: I'm somewhat surprised it doesn't give trouble with multiple wallets
< gwillen>
wumpus: ... actually it appears to be pretty broken with multiple wallets
< gwillen>
I never thought to try that
< gwillen>
it seems to keep UTXOs selected when switching wallets that don't exist in the second wallet
< gwillen>
I'm not sure what happens if I then try to spend them.
< gwillen>
perhaps this is the secret backdoor way to spend from multiple wallets at once, heh
< gwillen>
anyway I am tempted to fix it but I don't have spare cycles right now, ask me again in a month or two and I'll probably do it
< wumpus>
gwillen: please create an issue for it at least
< wumpus>
fanquake: thanks for keeping an eye on changes to the release notes-if this keeps up we'll have to restrict edits to people in the bitcoin-core org (I'd prefer not to as this loses some potential documentation-only contributors), but this may well be accidental and they fixed it
< gwillen>
wumpus: hmm, this may be either caused or revealed by my changes, investigating before I file it
< gmaxwell>
I've been watching logs on a couple nodes and it looks like new installs of 0.16.1 are significantly more common than later versions.
< gmaxwell>
I wonder if there is some outdated link someplace.
< wumpus>
maybe some distro did the cursed thing
< gmaxwell>
(or there is some spy/sybil-attacker that looks like these things, but I don't think so as I'm already pretty heavily excluding things that look like that)
< wumpus>
(e.g. freeze one version of bitcoin core as "stable")
< gmaxwell>
Checking my logs most of them appear to be IPs in china.
< gmaxwell>
jl2012: ^ any thoughts for this? is it possible some high visibility chinese language bitcoin resource links to 0.16.1 ?
< gmaxwell>
looks like 98% of them are chinese.
< jl2012>
I'll take a look
< jl2012>
Are they from random ip, or from a few datacenters?
< gmaxwell>
Many, though a large number are from 1.196.144.0/24
< gwillen>
wumpus: filed #15725, I can't make this happen every time but it's reliable enough that I'm convinced it's probably not anything I touched
< fanquake>
Also, anyone is upgrading to Xcode 10.2, you may have to re-install the macOS_SDK_headers.pkg, otherwise your depends builds may break.
< wumpus>
fanquake: this is very useful, thanks for working on that document
< wumpus>
I think we also need to document the exact steps to update the various subtrees, there's some overlap with the "Subtrees" section in doc/developer-notes.md btw
< fanquake>
wumpus Yea just documenting useful stuff. All the things I do infrequently enough that I have to go look it/the process up again.
< aj>
wumpus: i can't read numbers without thousands separators man
< luke-jr>
FWIW, it looks like Boost::Process 1.45 is broken in that it doesn't recongised died-by-signal as exited (and throws exceptions wildly as a result)
< luke-jr>
1.47 has it fixed I guess
< fanquake>
luke-jr Boost.Process in Boost 1.45 ? I thought it was only introduced in 1.64? Or are you talking about something else?
< MarcoFalke>
proposedmeetingtopic: TODOs in 0.18.0 Release Notes Draft
< MarcoFalke>
[05:41] <wumpus> I think we also need to document the exact steps to update the various subtrees, there's some overlap with the "Subtrees" section in doc/developer-notes.md btw
< gmaxwell>
jl2012: there isn't a chinese version of bitcoin.org or something that only shows old links? I'm just at a loss as to what would be causing people to start new nodes using old code.
< phantomcircuit>
gmaxwell, are they all on aws or something?
< midnightmagic>
gmaxwell: the TMSR people and their propaganda maybe?
< gwillen>
sipa or someone: are descriptors supposed to accept "h" anywhere they would accept "'"?
< gwillen>
gmaxwell: ^
< gwillen>
it seems like maybe there is an untested interaction with checksums
< gwillen>
I am finding that my descriptors with 'h' are being parsed as invalid
< gwillen>
if I getdescriptorinfo on a descriptor that uses 'h', I get back one that uses "'" and has a valid checksum, but if I then replace the ' with h and keep the checksum the same, the resulting descriptor is rejected
< sipa>
gwillen: yes, you can use either h or '
< sipa>
and the checksum will depend on which one you use
< gwillen>
hmmmmmm
< gwillen>
but getdescriptorinfo canonicalizes from "h" to "'" _before_ computing the checksum
< gwillen>
so I can't get one with h :-)
< sipa>
yes
< gwillen>
also it seems weird that they are treated differently, I would argue against that but maybe it's too late
< sipa>
gwillen: the alternative would require a checksum algorithm that's not black box
< gwillen>
it would just require that you always canonicalize before checksumming
< gwillen>
since it seems like the very first thing you do on parsing is canonicalize anyway
< sipa>
canonicalizing requiring understand the descriptors
< sipa>
while checksums can hopefully be performed by things that treat descriptors as strings
< sipa>
it was probably a mistake to permit both h and ' in the first place, but it's far too late for that
< gwillen>
this is somewhat sad for dev use, because the primary use of 'h' is not having to quote the apostrophe
< gwillen>
but there is no easy way to get a checksum on a descriptor that uses h
< sipa>
ah i see
< gwillen>
so in practice you always have to use the apostrophe
< gwillen>
which defeats the purpose of h existting
< sipa>
well you don't have to use getdescriptorinfo
< gwillen>
is there an RPC that will checksum a descriptor for me without altering it first
< sipa>
not an RPC, but there is python code for it
< gwillen>
can I argue for getdescriptorinfo not to alter the descriptor provided
< gwillen>
(is there any other issue like this aside from h/'?)
< sipa>
if you include a private key in the descriptor, the canonical version will drop it
< sipa>
because that's just syntactic sugar for the public version + simultaneously also conveying the private key
< gwillen>
except again for the checksum caring about the difference
< sipa>
yes
< gwillen>
I am heavily tempted to disable descriptor checksums locally while I'm doing development to make this more usable
< gwillen>
that is not a good sign :-P
< sipa>
it wouldn't be hard to add an RPC that just adds the checksum, but otherwise leaves the descriptor untouched
< sipa>
would that help?
< gwillen>
that would be helpful, but I also wonder if getdescriptorinfo really needs to canonicalize the descriptor
< gwillen>
but either one would do, yeah
< gwillen>
although I'm now building with descriptor checksums disabled anyway, since it was a one-line change
< sipa>
agree, perhaps it doesn't need to in the first place
< gwillen>
*nods*
< gwillen>
if the canonical version were 'h' rather than "'", it wouldn't require parsing to checksum the canonical version, since "'" can't appear anywhere else as far as I know
< gwillen>
(but this doesn't solve private keys, and also it's surely too late.)
< sipa>
a side effect of having the checksum depend on the exact string representation also means that 'canonical' version doesn't mean much
< sipa>
we could change the canonical encoding without breaking anything
< gwillen>
certainly h is easier to work with than "'"
< gwillen>
I don't know if it has any downsides I'm not thinking of
< sipa>
the ' notation is more widespread
< gmaxwell>
I don't think the rpc turning ' into h would be a problem. It does need to accept ' in inputs, of course.
< gwillen>
yeah, it should absolutely still accept both
< gwillen>
right now it turns h into '
< sipa>
wouldn't be hard to make it not touch it as well
< gwillen>
hm, I just did a ranged descriptor import with a range of 10,000
< gwillen>
and it seems to have hung
< gwillen>
it's been 120 seconds
< achow101>
I think having the canonical one be 'h' would be better
< sipa>
is it rescanning?
< gwillen>
I used {"rescan": false}
< sipa>
that's ungood
< gwillen>
I will attach a debugger
< achow101>
also having getdescriptorinfo give the checksum for descriptors with privkeys too
< achow101>
gwillen: I think that is not unexpected
< gwillen>
no?
< achow101>
IIRC it writes each generated key and key origin data independently without batching (I know, bad idea), so it takes a while due to disk I/O
< gwillen>
I mean, awhile is awhile, but it's now been like 5 minutes
< gwillen>
that is a long while to perform 10,000 writes
< gwillen>
and the gui thread is totally dead now
< gwillen>
before the GUI was responding but the console wasn't answering RPCs
< achow101>
hmm, that might be hung and not just taking awhile
< gwillen>
we are inside CWallet::CanGetAddresses
< achow101>
gwillen: oh, deadlock?
< gwillen>
oh, maybe, yeah, we're trying to lock a mutex
< achow101>
the gui thread probably went into cangetaddresses but can't acquire the lock so it
< sipa>
gwillen: is this 0.18/master or something with changes by you?
< achow101>
as the lock is held by the rpc thread for import
< gwillen>
sipa: changes by me
< gwillen>
not to any of this though I would expect?
< gwillen>
it looks like we may indeed be deadlocked trying to take cs_wallet at the top of CanGetAddresses
< gmaxwell>
What lock would be held before cs_wallet in the call path to CanGetAddresses? can you get a backtrace?
< gmaxwell>
(so we can figure out what the other lock involved in the inversion is?)
< sipa>
gwillen: compile with -DDEBUG_LOCKORDER
< sipa>
and try to reproduce
< gmaxwell>
I wonder if essentially anyone has used the GUI with -DDEBUG_LOCKORDER ?
< gwillen>
let me paste the backtrace first for gmaxwell
< gwillen>
I will try with debug_lockorder next
< sipa>
oh, the stack trace (for all threads) should actually tell you just as much
< gwillen>
there are a lot of threads, how do I backtrace threads in bulk in lldb
< gmaxwell>
thread apply all bt
< gmaxwell>
works in gdb, I assume lldb too
< gwillen>
sadly it does not work in lldb
< gwillen>
and I cannot gdb because mac
< sipa>
sadness
< gmaxwell>
thread backtrace all
< gmaxwell>
bt all
< gwillen>
aha
< gmaxwell>
^ lldb commands.
< achow101>
gwillen: can you check if your wallet file is being modified?
< gmaxwell>
(being modified while not paused in the debugger)
< gmaxwell>
GUI hanging in CanGetAddresses doesn't itself imply that there is a deadlock there. e.g. it could just be waiting on a lock that is deadlocked elsewhere, or not even locked but absurdly slow.
< gwillen>
hm yes, it is being modified
< gwillen>
it is growing.
< achow101>
gmaxwell: I'm pretty sure it's hanging in CanGetAddresses because cs_wallet is locked by the inport, which CanGetAddresses needs to acquire
< gwillen>
it's quite large.
< achow101>
no shit. that's 10000 keys
< gwillen>
how big is a key?
< gwillen>
this seems like support for "not even locked but absurdly slow"
< gmaxwell>
I was about to say that.
< achow101>
33 bytes + keyid a few times + metadata. probably like 80 bytes at least
< gmaxwell>
key records are on the order of 100 bytes I think.
< achow101>
i'm testing this locally. it looks like it spends a while on key derivation also
< gwillen>
does it also duplicate full key origin info for each?
< gwillen>
that would make it bigger.
< achow101>
yes
< gwillen>
we're at 4 megs and growing
< achow101>
not unexpected
< gwillen>
so it's definitely more than 80 bytes per.
< gmaxwell>
I'm gussing it's doing something stupid like opening and closing the wallet for each key.
< achow101>
gmaxwell: it's not using a batch write. using a batch write should speed it up significantly
< sipa>
gwillen: it absolutely is doing that
< gwillen>
even opening and closing the wallet 10,000 times shouldn't take 6 minutes, should it?
< sipa>
are you on an I/O starved VPS? :p
< gmaxwell>
gwillen: it writes and fsyncs each time.
< gwillen>
I guess if every time it opens it, it scans and validates a bunch of things, fires a bunch of notifications, etc. etc.
< gmaxwell>
and on mac there is no file-centric fsync.
< achow101>
working on fix
< sipa>
achow101: nice
< sipa>
i guess some of the wallet encryption logic can be used to avoid opening/closing the db every time?
< gwillen>
will my testnet wallet get rekt if I murder this process in the middle of whatever it's doing
< gmaxwell>
We had this same problem with the original wallet creation at one point, IIRC
< gwillen>
sipa: what's the deal with wallet compression, is it an option?
< sipa>
gwillen: if you never encrypted the wallet it's unencrypted
< gwillen>
erm, you said compression before, but this time you said encryption
< sipa>
i meant encrypted :)
< gwillen>
oh, heh
< gwillen>
does encryption also compress
< * sipa>
falls asleep
< gwillen>
or is this not actually relevant to the problem of walletfile being huge
< gwillen>
you said something about 270 bytes per key
< gwillen>
I would have assumed default compression, especially if we're repeating mostly-redundant key origin info for every key
< gwillen>
that should be extremely compressible
< gwillen>
my wallet has reached 8.5 megs
< sipa>
gwillen: the old unencrypted wallet format stores private keys in a ricldiculous der encoded format of 279 bytes per key
< sipa>
where only 32 bytes are needed
< gwillen>
ah, well this wallet should be relatively new, like within the last few months
< sipa>
not relevant
< sipa>
the encoding used for unencrypted keys is still that der one
< gwillen>
also this is watchonly so there are no private keys anyway
< sipa>
oh!
< sipa>
then this is all irrelevant
< gwillen>
so presumably all that space is being taken up by key origin info and other metadata
< gwillen>
although that's still more than 1kb per key, which seems huge
< gwillen>
achow101: how large did your wallet get after you spent 6 minutes importing 10,000 keys
< sipa>
gwillen: doesn't surprise me
< achow101>
gwillen: 8 MB, but I also did it again and now it's 16 MB
< gwillen>
heh
< gwillen>
so mine was almost done probably
< gwillen>
doing 100 takes 3 seconds on my machine, so 10000 would have taken about 5 minutes to finish
< achow101>
well batching writes reduced that from 6:56 to 1:24
< gwillen>
ha
< bitcoin-git>
[bitcoin] achow101 opened pull request #15741: Batch write imported stuff in importmulti (master...batch-write-importmulti) https://github.com/bitcoin/bitcoin/pull/15741
< achow101>
gwillen: ^^
< gwillen>
:+1: will review!
< gwillen>
achow101: is this kind of manual batching typical of using WalletBatch? I wonder if it would make sense to have a self-flushing WalletWriteCache or something
< gwillen>
to avoid having to have all the fiddly counting of entries
< achow101>
gwillen: we do something similar for initial key generation
< achow101>
i was mostly following upgradekeymetadata (aka copy paste) which does counting
< gwillen>
would be good to factor it out nicely ;-)