< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #17365: depends: update README.md with working Android targets and API levels (master...update-android-depends-doc) https://github.com/bitcoin/bitcoin/pull/17365
< bitcoin-git>
[bitcoin] jnewbery opened pull request #17383: Refactor: Move consts to their correct translation units (master...2019-11-net-processing-consts) https://github.com/bitcoin/bitcoin/pull/17383
< achow101>
The main weird thing was this inconsistency I just noticed where you could import a wpkh() descriptor and still get metadata for the legacy address that shares the same pubkey with it, but that legacy address is ismine_no
< instagibbs>
that's fun
< achow101>
and I don't think we can/should retain this behavior in ScriptPubKeyMan, but that's also a breaking change
< wumpus>
I'm tempted to just drop that commit
< wumpus>
ryanofsky: testnet=0 is fine, notestnet=1 is not
< wumpus>
it's the double negation that messes things up
< instagibbs>
achow101, this is only for pkh detection?
< achow101>
maybe?
< ryanofsky>
wumpus, feel free to drop the commit, i can follow up later if needed. anyway i'm going to debug
< achow101>
probably. I think it might not detect some more esoteric stuff like p2pkh inside of p2sh
< instagibbs>
right, nested checks
< ryanofsky>
to be sure though, -notestnet=1 is not double negation, that's just single negation
< wumpus>
ryanofsky: there's no hurry but if people really insist on keeping the current crazy behavior I'm not going to argue :)
< instagibbs>
to be clear, you're saying 17374 is a breaking change
< achow101>
yes. and so is the final ScriptPubKeyMan refactor
< wumpus>
ryanofsky: right, notestnet=0 would be a double negation
< wumpus>
I remember support for that was dropped at some point, notestnet=1 probably should just ignore the parameter
< instagibbs>
achow101, I find this corner case very surprising from our dumb ismine current wallet fwiw
< achow101>
instagibbs: well breaking in different ways. 17374 is breaking by changing ismine behavior to allow more things. scriptpubkeyman pr would be break getaddressinfo behavior without 17374
< instagibbs>
ok im just considering 17374.
< sipa>
achow101: i don't think losing signingprovider info for non-ismine things is a big problem
< sipa>
worth a mention in release notes
< achow101>
sipa: right, I'm leaning that way too. the thing that confused me was that we have a few tests that rely on that behavior, probably on accident
< wumpus>
hehe
< instagibbs>
ack
< wumpus>
ryanofsky: looking at it, it's very possible that that test is broken and not the argumet parsing in general
< ryanofsky>
yes, it looks like the multiple config entries created by the test are stuck together
< wumpus>
right
< bitcoin-git>
[bitcoin] achow101 closed pull request #17374: IsMine: Set state to WATCH_ONLY if we can get the pubkey (master...wallet-box-pr-7) https://github.com/bitcoin/bitcoin/pull/17374
< wumpus>
it's not the most easy to understand code
< wumpus>
like using "desc += argstr + 1;" to add to a string, yes you can write C++ code like that, but...
< luke-jr>
lol
< bitcoin-git>
[bitcoin] achow101 opened pull request #17387: wallet_importmulti: use addresses of the same type as being imported (master...tests-fix-getaddrinfo-type-consistency) https://github.com/bitcoin/bitcoin/pull/17387
< wumpus>
so I *think* the idea would be to add a new line in between there
< wumpus>
yeh adding conf += "\n"; at the end of that loop stops the 1testnet=1 problem
< wumpus>
but it causes the test to fail, likely because of a wrong expectation
< wumpus>
at least it fails in the same way with and without my patch after that :-)
< ryanofsky>
thanks, yeah, pushing a pr in a minute or two, it's unrelated to your pr
< wumpus>
-regtest=0 -noregtest=1 testnet=1 regtest=1 || test -> error: Invalid combination of -regtest, -testnet and -chain. Can use at most one.
< wumpus>
definitely better
< wumpus>
I don't think anyone reviewed the previous test output ever
< wumpus>
thanks
< bitcoin-git>
[bitcoin] ryanofsky opened pull request #17388: Add missing newline in util_ChainMerge test (master...pr/chainmerge-nl) https://github.com/bitcoin/bitcoin/pull/17388
< meshcollider>
achow101: what kind of tests rely on that behaviour?
< achow101>
the importmulti ones
< achow101>
trivial to make them not rely on it, see the pr I just opened
< bitcoin-git>
[bitcoin] RandyMcMillan opened pull request #17389: build: add background.tiff and .pngs to "make clean" (master...make-clean-tiff) https://github.com/bitcoin/bitcoin/pull/17389
< wumpus>
it's pretty strange to have a one-time boost::interruption point at the beginning of BerkeleyEnvironment::Open and BerkeleyEnvironment::BerkeleyEnvironment, seems like something the caller could handle if needed
< wumpus>
these are meant for being able to interrupt long-running operations