< luke-jr> gmaxwell: "new"? didn't we merely just remove a long-deprecated way?
< sipa> luke-jr: to many people it seems new :)
< sipa> perhaps instead of "It is recommended to use a new receiving address for each transaction" it should say "Use the request payment button in the receive tab to create new addresses."
< meshcollider> yep and these are good hacktoberfest tag issues :)
< gmaxwell> luke-jr: "new address" process, not new "address process"
< bitcoin-git> [bitcoin] ken2812221 opened pull request #14480: refactor: Drop boost::this_thread::interruption_point and boost::thread_interrupted in main thread (master...drop-boost-thread-import) https://github.com/bitcoin/bitcoin/pull/14480
< bitcoin-git> [bitcoin] MeshCollider opened pull request #14481: Add P2SH-P2WSH support to listunspent RPC (master...201810_listunspent_wsh) https://github.com/bitcoin/bitcoin/pull/14481
< bitcoin-git> [bitcoin] MeshCollider closed pull request #11708: Add P2SH-P2WSH support to signrawtransaction and listunspent RPC (master...201711_signrawtransaction_wsh) https://github.com/bitcoin/bitcoin/pull/11708
< karelb> Is there some linter on bitcoin core that looks for line length?
< luke-jr> karelb: hopefully not, as there is no line length limit? :p
< karelb> :) ok. Some linters say "ok lines should not be longer than 80 characters" etc
< karelb> s/some linters/some best practices/
< luke-jr> in practice, we do have some hard wrapping in the codebase, but IMO it's a bad idea
< sipa> we have plenty of lines that are longer :)
< luke-jr> everyone's editor is a different width, and can soft-wrap as needed
< sipa> and we do have a suggested coding style defined by the clang formatter config in the repo
< karelb> ok, just asking. :D I am now refactoring the RPC doc stuff in this vein - https://gist.github.com/karel-3d/5847ea0172350368dead323211893faa#file-rpc_doc-cpp-L244 - so I am thinking if I should also add some line length limit
< sipa> but it's not really possible to enforce a strict style without making people waste time dealing with every tiny nit
< sipa> karelb: make sure you follow conventions for variable/class/... names in new code
< meshcollider> karelb: is this related/the same/similar to what achow101 is working on
< sipa> (see developer-notes.md)
< meshcollider> that is, do both of you know what the other is working on to make sure it doesn't overlap
< karelb> meshcollider: I think it is slightly different. I have saw his repo and I did not really understand it all that much, but it seemed complementary
< meshcollider> ok, as long as you're aware of it :)
< karelb> I am, I just don't really understand the code :(
< meshcollider> achow101: ping
< karelb> Hm it seems it is actually similar to what I did, but using univalue
< karelb> sipa: thanks for the link to dev-notes. I will use that
< karelb> (I did not write any big c++ code in years, I am surprised it works)
< meshcollider> lol
< * sipa> learned c++ from the bitcoin codebase
< sipa> i guess it explains some things...
< karelb> :D
< karelb> "Class member variables have a m_ prefix" - I don't see that in many class variables in bitcoin codebase?
< luke-jr> karelb: old code isn't changed, just new code is expected to follow these
< karelb> oh ok
< karelb> so it's not a good idea to look at existing code for reference
< meshcollider> no there are so many styles everywhere it is very inconsistent
< karelb> ooooh ok
< meshcollider> e.g. new class naming convention is to not start the class name with C iirc, but most existing classes start with C like CWallet
< sipa> karelb: read the first parafraph
< sipa> *paragraph
< karelb> Oooooh ok. Hmm, looking in github on src/ history, it's interesting how little is there new classes added and mostly it's fixes of existing code. Well not that surprising actually.
< sipa> yeah :)
< sipa> karelb: also, new code is often written in new files
< karelb> Also - how much does bitcoin (at least new code) use all the const correctness stuff? I never know how to write it correctly and where to add `const`
< bitcoin-git> [bitcoin] HatboyWonder opened pull request #14484: changed request payment button text and tab description (master...master) https://github.com/bitcoin/bitcoin/pull/14484
< sipa> karelb: const correctness is easy, never add a const cast
< sipa> if it compiles, you're good
< sipa> :)
< sipa> and there are some small parts of the code that are not const correct, but mostly, yes
< sipa> (in particular the serialization code does some hairy stuff)
< meshcollider> sipa: I was thinking about the import descriptor thing internally converting to old structures
< meshcollider> for ranges, how would that work
< meshcollider> surely not cover the whole 2^31-1 range or whatever?
< meshcollider> or is that ok
< meshcollider> or just not support ranges for now
< sipa> meshcollider: you'd specify the range along with the import
< sipa> if you specify a billion, you're importing a billion addresses, and the wallet file will likely catch fire and explode
< meshcollider> only one wildcard is allowed in the path eh? No ambiguity if only a single number is used to specify the range?
< sipa> you can have multiple hd paths that end with * in a descriptor
< sipa> but they're always combined pairwise
< sipa> or in other words, all *s are replaced with the same number in an expansion
< meshcollider> ah I see, yep
< meshcollider> and does a range always start from 0
< meshcollider> or should it be a start, end pair
< sipa> that's up to the application
< sipa> the descriptor is really just a list of addresses
< sipa> the application chooses to evaluate it at certain positions of the list
< bitcoin-git> [bitcoin] jonasschnelli pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/be992701b018...2a2cac787360
< bitcoin-git> bitcoin/master 2f6b466 Pieter Wuille: Stop requiring imported pubkey to sign non-PKH schemes
< bitcoin-git> bitcoin/master 2a2cac7 Jonas Schnelli: Merge #14424: Stop requiring imported pubkey to sign non-PKH schemes...
< bitcoin-git> [bitcoin] jonasschnelli closed pull request #14424: Stop requiring imported pubkey to sign non-PKH schemes (master...201810_importpubkeylol) https://github.com/bitcoin/bitcoin/pull/14424
< luke-jr> MarcoFalke: why'd you close #14080 ?
< gribble> https://github.com/bitcoin/bitcoin/issues/14080 | travis: Run unit tests --with-sanitizers=thread by MarcoFalke · Pull Request #14080 · bitcoin/bitcoin · GitHub
< jonasschnelli> sipa: what is the status of Bech32X (26 char checksum BCH)?
< bitcoin-git> [bitcoin] luke-jr opened pull request #14485: Try to use posix_fadvise with CBufferedFile (master...fadvise) https://github.com/bitcoin/bitcoin/pull/14485
< bitcoin-git> [bitcoin] mrwhythat closed pull request #14476: RPC method 'encodescript' (master...encodescript-rpc) https://github.com/bitcoin/bitcoin/pull/14476
< vamp111> Hi, where can i write to support, i have some issues with my full node
< belcher> vamp111 #bitcoin is best
< meshcollider> vamp111: either #bitcoin or use the stack exchange, bitcoin.stackexchange.com
< meshcollider> sipa: how does something like this look https://github.com/MeshCollider/bitcoin/tree/201810_importmulti_desc
< meshcollider> Maybe just look at the test case rather than the whole diff for a concept ack
< meshcollider> I'll open a PR
< bitcoin-git> [bitcoin] DesWurstes opened pull request #14486: Add explicit cast to base58 and bech32 string constants in order to silence GCC warning (master...patch-4) https://github.com/bitcoin/bitcoin/pull/14486
< bitcoin-git> [bitcoin] DesWurstes opened pull request #14487: Constexpr Everything Part 1: Constants (master...patch-3) https://github.com/bitcoin/bitcoin/pull/14487
< instagibbs> meshcollider, oh did you start on the importmulti for descriptors? I already had, but feel free to do it :)
< echeveria> that's binance out of tether.
< echeveria> they've disabled their tether wallet.
< waxwing> sipa, is this comment actually valid as of now? https://github.com/bitcoin/bitcoin/blob/0.17/src/script/sign.cpp#L252-L255
< waxwing> i ask because FillPSBT won't allow both as per https://github.com/bitcoin/bitcoin/blob/0.17/src/wallet/rpcwallet.cpp#L4528-L4534
< waxwing> and non-wallet inputs would be covered by the deserialization checking sanity (as i think it says in the comment)
< promag> jnewbery: can you review #14291?
< gribble> https://github.com/bitcoin/bitcoin/issues/14291 | wallet: Add ListWalletDir utility function by promag · Pull Request #14291 · bitcoin/bitcoin · GitHub
< sipa> waxwing: not valid anymore, right
< sipa> the code in rpcwallet you quote is more recent
< waxwing> sipa, thx
< sipa> achow101, meshcollider: looking back at your PRs #14454 and #14019, i'm confused why we need to import the raw pubkey scripts when doing an importmulti
< gribble> https://github.com/bitcoin/bitcoin/issues/14454 | Add SegWit support to importmulti by MeshCollider · Pull Request #14454 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/14019 | Import pubkeys when importing p2sh with importmulti by achow101 · Pull Request #14019 · bitcoin/bitcoin · GitHub
< sipa> i understand you may want to add a key to mapKeyMetaData in order to have hd path information etc, but that's not what's happening
< sipa> and since #14424 having the pubkey itself imported should only be needed to P2PKH and P2WPKH
< gribble> https://github.com/bitcoin/bitcoin/issues/14424 | Stop requiring imported pubkey to sign non-PKH schemes by sipa · Pull Request #14424 · bitcoin/bitcoin · GitHub
< achow101> sipa: 14019 as needed as a precursor to #14021 which does the hd path import stuff.
< gribble> https://github.com/bitcoin/bitcoin/issues/14021 | Import key origin data through importmulti by achow101 · Pull Request #14021 · bitcoin/bitcoin · GitHub
< achow101> I separated that into 2 PRs because the importing of pubkeys in a p2sh seemed to be orthogonal to the end goal
< sipa> but why do you need the pubkey itself imported?
< sipa> i think you just need a mapKeyMetadata entry
< achow101> so that GetPubKey and/or CreateSig work
< sipa> GetPubKey should only be needed for PKH schemes
< sipa> since 14424
< sipa> i'd really like to avoid making unrelated things watched; for multisig that's even dangerous as someone can trick you into thinking you're being paid by paying to one of the constituent pubkeys rather than the multisig script
< sipa> oh, CreateSig also takes a CKeyID as input?
< sipa> oh, no
< sipa> not anymore
< sipa> achow101: can you please test if this is still needed since 14424?
< achow101> with 14424, I don't think that importing pubkeys with p2sh is necessary anymore
< sipa> okay!
< achow101> unless we support signing with arbitrary scripts which may contain pubkey hashes
< achow101> (this has been requested by someone and I said I would look into it)
< sipa> that's not supported anyway right now
< sipa> and with descriptors that would become possible without making payments to those pubkeys themselves treated as ismine
< achow101> eh, the request was actually signing arbitrary scripts in a psbt, so importing pubkeys probably wouldn't be necessary
< achow101> It could probably be done by bypassing ProduceSignature and doing a simple signer instead for psbts
< sipa> right
< bitcoin-git> [bitcoin] achow101 closed pull request #14019: Import pubkeys when importing p2sh with importmulti (master...import-multi-pubkeys) https://github.com/bitcoin/bitcoin/pull/14019
< meshcollider> sipa: in that case, it looks like I can just delete the entire "// Import public keys." block of code?
< meshcollider> Because the case of single key addresses P2PK, P2PKH, P2WPKH is already covered above
< meshcollider> Well, I just need to move the actual import to there
< sipa> meshcollider: i haven't looked in detail at the implementation
< meshcollider> I mean, is the only case where we need to import a public key is for a P2PKH/P2WPKH without the private key?
< sipa> right, and P2SH-P2WPKH
< bitcoin-git> [bitcoin] ken2812221 opened pull request #14489: refactor: Drop boost::thread and boost::chrono (master...interruptible-thread) https://github.com/bitcoin/bitcoin/pull/14489