< wumpus>
loooooks like travis is acting up again (in #15324 at least): /home/travis/.travis/job_stages: line 104: .travis/test_06_script.sh: No such file or directory
< wumpus>
gkrizek: "[bitcoin] ... (.17...019-02-backport-release-notes)" looks like the first letter of the branch names is missing ! :)
< gkrizek>
Yeah I noticed that. I think it only happens on the branch names with ‘0.X’. I’ll look into it
< wumpus>
gkrizek: maybe always when it starts with a number
< wumpus>
the 019-02... one looks cut-off too
< gkrizek>
Ah, could be!
< fleshwounded>
woot woot found the @wumpus
< fleshwounded>
you really dont leave the irc do you lol?
< fleshwounded>
hope that router is doing some fancy stuff now? you plug an pcie in? or what was is again msata I forget lol... been eye humping those routers ever since we talked lol
< fleshwounded>
I wanna do the satellite node now...
< wumpus>
fleshwounded: my IRC runs on a server that's always on (weechat on tmux right now), this is useful to not miss backlog
< wumpus>
fleshwounded: and feel free to PM me about the other things, this is off topic for the channel
< bitcoin-git>
[bitcoin] practicalswift opened pull request #15327: tests: Make tests updatecoins_simulation_test and knapsack_solver_test deterministic (master...SeedInsecureRand(true);) https://github.com/bitcoin/bitcoin/pull/15327
< fleshwounded>
I just wasnt sure if I was in the right place or not @wumpus not a problem lol
< wumpus>
ok !
< MarcoFalke>
re travis failures: Not all pull requests need rebase (only those that failed prior to now and need to be run again). A force push or push should also suffice, though.
< provoostenator>
Any idea what I should cherry-pick to prevent "Assertion failed: (!m_result.empty()), function RPCResult, file ./rpc/util.h, line 118." ?
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #15328: travis: Revert "Run extended tests once daily" (master...Mf1902-travisExt) https://github.com/bitcoin/bitcoin/pull/15328
< MarcoFalke>
provoostenator: It tells you to not pass in an m_result that is empty
< MarcoFalke>
All rpc have a result (even if it is Null)
< provoostenator>
It crashes for src/bitcoin-cli help
< provoostenator>
During the block rewind phase "bitcoin-cli help" complains as usualy that it's still rewinding. After that it crashes. Same if I use the console in QT.
< provoostenator>
If nothing comes to mind though, I'm just going to go through the misery of rebasing and see if that magically helps :-)
< provoostenator>
achow101 / meshcollider: if you get a chance to rebase #14491 that would be quite helpful. I can't figure it out.
< provoostenator>
"bitcoin-cli help" causes a reliable crash
< provoostenator>
But not on master
< provoostenator>
So I figured that I probably created too much of a nested PR mess and need to start again from master, but that's not an option pending that rebase.
< meshcollider>
So somewhere in those 36 commits theres a bug, yay!
< provoostenator>
Maybe, and it's possible I didn't notice it earlier today because some cached thing saved me.
< meshcollider>
Ok I'll try and get it done today or tomorrow at the latest but like you say, its a bit of a headache to rebase this one
< provoostenator>
Meanwhile I'll try nuking some things on my end to see if I can get things to compile
< meshcollider>
I'll have to base it on top of sipas PR so then we have yet another PR in the tower
< provoostenator>
It'd say the descriptor import PR is the scariest thing in the tower. Should be smooth sailing once that's merged...
< provoostenator>
Because it's both a big refactor and adding new functionality.
< provoostenator>
And every time we find a problem, we've been adding some parallel PR to deal with that.
< provoostenator>
I'd also love to move most of that stuff out of the RPC codebase and into a place where it can also be used by the GUI, but let's not for now :-)
< meshcollider>
yep lets do a move PR after the water clears a bit :)
< provoostenator>
#14978 is a good example of moving stuff out of RPC codebase
< gribble>
https://github.com/bitcoin/bitcoin/issues/14978 | Factor out PSBT utilities from RPCs for use in GUI code; related refactoring. by gwillen · Pull Request #14978 · bitcoin/bitcoin · GitHub
< meshcollider>
actually is anyone around e.g. jnewbery that could quickly review #15263
< provoostenator>
So I need to do RPCResult{"null"} for entries without no result doc?
< bitcoin-git>
bitcoin/master 11e0fd8 Pieter Wuille: Descriptor expansions only need pubkey entries for PKH/WPKH
< bitcoin-git>
bitcoin/master 6e6b859 MeshCollider: Merge #15263: Descriptor expansions only need pubkey entries for PKH/WPKH
< bitcoin-git>
[bitcoin] MeshCollider merged pull request #15263: Descriptor expansions only need pubkey entries for PKH/WPKH (master...01901_flatprovider_pkh) https://github.com/bitcoin/bitcoin/pull/15263
< MarcoFalke>
Yes, I believe we should document that the return value is json-null instead of ommiting it
< MarcoFalke>
* omitting
< provoostenator>
MarcoFalke: same for RPCExamples?
< MarcoFalke>
Not required, up to you to provide examples
< provoostenator>
Crash gone! Thanks.
< meshcollider>
\o/
< provoostenator>
meshcollider: I see you merged it :-) Maybe add my "Test importing of a P2PKH address via descriptor" when you rebase?
< meshcollider>
provoostenator: will do, thanks
< provoostenator>
No rush though, with the crash out of the way at least I'm not stuck. Though I'd still like to get that the descriptor import multi PR merged, to pave the way to get achow101's PR's merged.
< meshcollider>
yeah I agree
< meshcollider>
these stacked PRs make things so much harder
< provoostenator>
With a bit of luck we'll have everything to use HWI Python scripts in standalone mode before 0.18
< hebasto>
wumpus: regarding #15329; what about translation of errors related to wallets or invalid option values?
< wumpus>
hebasto: I don't know, don't have a strong opinion on it; but note that we also don't translate option help
< wumpus>
hebasto: in any case I think the criterion for translation should be "does this message make sense to a user/translator", you can assume knowledge of bitcoin terminology such as blocks, transactions, wallets, etc, but not internals of the software
< wumpus>
so for the wallet messages it makes more sense to translate I guess
< hebasto>
^^ that was my initial intention
< wumpus>
somthing like "wallet cannot be loaded" is very translatable
< gmaxwell>
Doesn't translation of errors make googling for help impossible?
< wumpus>
but something highly technical, like, "wallet premature EOF truncated key" you'd certainly not want to translate
< wumpus>
gmaxwell: yes that was exactly my point in #15329
< wumpus>
I mean you could also say "don't translate any errors at all" but that'd be a new policy, in fact some InitError and InitWarning are translated at the moment
< wumpus>
it's fine with me though ...
< gmaxwell>
It might be more useful to show both the translated and english error, but I dunno if the translation infrastructure makes that possible
< gmaxwell>
otherwise I think we're stuck wondering if the error is user actionable ("wallet file not found") vs not ("hyperspatial cascade failure in wallet")
< wumpus>
yes
< hebasto>
for future maintenance purpose is it good to explicitly comment untranslated errors?
< wumpus>
so I think the first point here should be: do we actually have a problem? is this a serious issue?
< gmaxwell>
hebasto: I think most errors outside of UI components are intentionally not translated.
< wumpus>
gmaxwell: yes
< wumpus>
it's only the UI that does translation, in any case
< hebasto>
InitError() presents error via UI
< echeveria>
I've seen users turn up with translated error messages before. it wasn't useful to me as someone who does not know russian, and they being bilingual and having to translate it back for me.
< wumpus>
It might be more useful to show both the translated and english error -> this is not possible at the moment, would be possible to create a _() alternative that returns both the original string and the transalation I guess, but that'd need changes to a few scripts
< wumpus>
(e.g. gettext needs to pick it up)
< wumpus>
echeveria: yeah... it's terrible
< gmaxwell>
I would guess that this is one reason that some commercial packages have numbered error messages.
< wumpus>
that works :) though the usual way is to make errors an enum, per subsystem, e.g. 1xxx is block chain error, 2xxx wallet error and so on, this way having the error strings in the code can be avoided completely, they can be in some text/database file including translations ...
< gmaxwell>
[SHA256] PATCH: adjust round constants to preserve E1132 with new text.
< wumpus>
hehe
< gmaxwell>
lets just make _() steg encode the line number into case alterations in the text... "A légpáRnásOM tELe vaN anGolnákKal."
< wumpus>
or add random data to the end of the error message and grind
< * sipa>
writes gramtropy grammar
< gmaxwell>
oh bitcoin error messages grammar.
< wumpus>
or that
< sipa>
gmaxwell: among degree 7, length 32767, distance 4 codes there exist only 1800 unique ones; among those, there is one with length 172 distance distance 5
< gmaxwell>
sipa: sadly not long enough for the 'P2WSH 2-of-3 multisig' example in the descriptors docs.
< sipa>
yup
< sipa>
i can look at 8 character codes
< gmaxwell>
Worth taking a look.
< gmaxwell>
5 up to 172 is still better than just distance 4... though I'd be particularly satisfied if it was long enough for 2 of 3 with hex pubkeys. (about 221-ish?)
< sipa>
209
< gmaxwell>
209 is without wsh() and without the checkvalue itself.
< sipa>
though 280 if we have 3-to-4 expansion for the character classes inline
< sipa>
ah yes!
< gmaxwell>
if we have 3-to-4 characters there will be a base-58 check though.
< sipa>
we can also have all character class 'symbols' first and then all the individual character symbols, in which case the length only applies whenever errors within the class are made (which is very likely for the multisig with hex pubkeys example; it only excludes errors in the wsh/multi keywords)
< gmaxwell>
jinx I was just thinking about that.
< gmaxwell>
But it wasn't completely clear to me if it was possible without having placeholder characters.
< gmaxwell>
or without a weird integration with parsing.
< sipa>
there are 94 printable ascii characters, so you only need 3 character classes
< gmaxwell>
like... would you process this by parsing to strip all the base-58 parts then check them then apply the checksum to the base58parts then the stripped part?
< sipa>
no, with every character you assign a character class and a 5-bit position within that class; and then you transform every 3 characters into 4 symbols, where the first symbol is cls1*9+cls2*3+cls3, and the other 3 are the individual positions of the 3 characters in their class
< sipa>
ah i see what you're getting at
< sipa>
you could actually sort all index positions based on their class
< sipa>
so then as long as all errors are within the same class, the distance is excluding any intermediary characters in other classes
< gmaxwell>
but an error that changes a base58 character to non-base58 than shifts all the non-base58 characters.
< gmaxwell>
e.g. a one character error could be undetected.
< sipa>
ah yes
< sipa>
that would be unfortunate
< gmaxwell>
thats why I was musing about parsing to split.
< sipa>
let's see what distance we can do with degree 8