< phantomcircuit> sipa, sort of
< phantomcircuit> sipa, switching to unordered_map had basically no effect
< bitcoin-git> [bitcoin] kazcw opened pull request #14734: fix an undefined behavior in uint::SetHex (master...SetHex-bad-ptr) https://github.com/bitcoin/bitcoin/pull/14734
< phantomcircuit> sipa, moving the buildpollfd logic into another method made the performance impact of socketevents disappear
< phantomcircuit> im confused
< phantomcircuit> sipa, oh the profile is messed up by importing mempool
< Dizzle> sipa or anyone that might know... does TestBlockValidity end up adding new valid blocks to the chain?
< Dizzle> It calls CChainState::ConnectBlock, but I couldn't figure out if that actually adds the new block to the chain
< Dizzle> Or morcos?
< achow101> sipa: can you rebase #14565?
< gribble> https://github.com/bitcoin/bitcoin/issues/14565 | Overhaul importmulti logic by sipa · Pull Request #14565 · bitcoin/bitcoin · GitHub
< achow101> so we can rebase all the things on top of it
< luke-jr> Dizzle: it doesn't, and generally isn't expected to be called with mined blocks
< luke-jr> Dizzle: note it does not perform the PoW check
< Dizzle_> luke-jr: if the mined block is proposed to Bitcoin Core via getblock template, it gets called, and I couldn't find an indication that a valid proposal gets submitted by Core.
< Dizzle_> *getblocktemplate
< Dizzle> I'm assuming a pool implementing gbt would submit a valid proposal to their p2p and fibre peers?
< luke-jr> Dizzle: mined blocks aren't supposed to be proposed in that manner, they're supposed to be fed to submitblock
< luke-jr> a pool could, but clients shouldn't assume they will
< luke-jr> Dizzle: proposals are for checking that the block is valid BEFORE mining it
< Dizzle> luke-jr: ahhh, thanks! That was the missing piece of the puzzle for me. I was thinking it was like getwork(data)
< luke-jr> did you read the BIP? <.<
< Dizzle> I did. It didn't really define what a proposal was, just how to construct one
< luke-jr> hmm
< Dizzle> Not your fault. I didn't jump into this knowing mining especially well. So proposal is for something like, "If I were to change the coinbase to spend coins to myself, would you accept the mined block when I submit it?"?
< luke-jr> Dizzle: if the BIP is unclear, it is indeed my fault :P
< luke-jr> Dizzle: in bitcoind's case, you have to make the coinbase from scratch, but otherwise yes
< Dizzle> Sweet. Thanks! Heading out for the night.
< gmaxwell> luke-jr: I guess it wouldn't be bad for it to actually accept it, if it turned out to be valid.
< luke-jr> no, other than setting up a false expectation
< provoostenator> Is there (or should there be) a document in the repo or elsewhere to describe best practices for replacing deprecated RPC calls?
< provoostenator> E.g. someone asked me what to use instead of signrawtransaction (answer: signrawtransactionwithkey or signrawtransactionwithwallet).
< wumpus> for 'getinfo' we had a special message for a while, if you used the old RPC call, it'd give an error message to use the new ones, I think that would be good practice for other removed RPC calls as well at least for one major release
< instagibbs> Is there a great cost to just leaving around a disabled message indefinitely? No supporting code infra etc.
< instagibbs> Doesn't even have to be verbose, just "disabled, see release notes version blah" or something
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/384967f311b4...35739976c1d9
< bitcoin-git> bitcoin/master 07e286d Carl Dong: Improve scripted-diff developer docs...
< bitcoin-git> bitcoin/master 3573997 MarcoFalke: Merge #14731: doc: Improve scripted-diff developer docs...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #14731: doc: Improve scripted-diff developer docs (master...patch-4) https://github.com/bitcoin/bitcoin/pull/14731
< phantomcircuit> anybody else had issues getting -pg to work?
< phantomcircuit> i get a gmon.out file but gprof analysis is empty
< phantomcircuit> apparently gcc 6 doesn't work with pie and pg
< phantomcircuit> sad
< zallarak> Is there any reviewer here that is willing to provide feedback on a PR that adds a flag to bitcoind?
< sipa> zallarak: what PR?
< bitcoin-git> [bitcoin] kristapsk opened pull request #14738: Fix running wallet_listtransactions.py individually through test_runner.py (master...listtransactions) https://github.com/bitcoin/bitcoin/pull/14738
< zallarak> Am I clear in understanding that all new code should be in snake case, even if surrounding code is camel case?
< sipa> zallarak: correct (for variable names)
< zallarak> Thank you, updating my PR for that.
< sipa> sorry, i'm going to be 10-15 minutes late for the wallet meeting
< instagibbs> #timeforwalletmeeting
< instagibbs> or we can wait until pieter is here
< provoostenator> I can wait 15 mins...
< gwillen> :+1:
< instagibbs> ack
< meshcollider> I probably won't be able to be here the whole time, leaving in around half an hour
< achow101> wallet meeting?
< meshcollider> achow101: See messages above :p
< sipa> back.
< sipa> #startmeeting
< lightningbot> Meeting started Fri Nov 16 19:15:03 2018 UTC. The chair is sipa. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< sipa> topics?
< provoostenator> Not really, I need to catch up on review and testing.
< instagibbs> #14565 needs rebase/more review
< gribble> https://github.com/bitcoin/bitcoin/issues/14565 | Overhaul importmulti logic by sipa · Pull Request #14565 · bitcoin/bitcoin · GitHub
< meshcollider> Hi
< sipa> yeah, i need to get back to that
< meshcollider> Yeah I need to rebase #14491 on it to
< gribble> https://github.com/bitcoin/bitcoin/issues/14491 | Allow descriptor imports with importmulti by MeshCollider · Pull Request #14491 · bitcoin/bitcoin · GitHub
< sipa> and i need to add more tests
< sipa> and address ryanofsky_'s comments
< instagibbs> gwillen, any update on psbt signing stuff?
< provoostenator> For those reading (the logs): ##hwi is a fun place for those interesting in _hardware_ wallet support, and there's some overlap with this effort.
< sipa> a question i had, how do we progress with things like #14481 ?
< gribble> https://github.com/bitcoin/bitcoin/issues/14481 | Add P2SH-P2WSH support to listunspent RPC by MeshCollider · Pull Request #14481 · bitcoin/bitcoin · GitHub
< gwillen> instagibbs: no update, other than I think https://github.com/bitcoin/bitcoin/pull/14588 getting in happened since the last time we had this meeting
< sipa> because it seems to me that using descriptors instead of lists of keys/redeemscript/witnessnessscript in sign* RPCs will be a much better user experience
< gwillen> I still owe another PR of PSBT related refactoring, and I have a working demo of offline signing UI if anybody wants to play with it (it's the same one I had around this time last week already)
< provoostenator> How would listunspent return descriptors?
< sipa> provoostenator: #14477
< sipa> already implemented
< meshcollider> provoostenator: accept, not return
< gribble> https://github.com/bitcoin/bitcoin/issues/14477 | Add ability to convert solvability info to descriptor by sipa · Pull Request #14477 · bitcoin/bitcoin · GitHub
< sipa> meshcollider: no, return
< instagibbs> the stuff you need to pass to other calls
< provoostenator> Ah ok, by converting scripts, that makes sense.
< meshcollider> Oh right
< provoostenator> And ultimately if the wallet contains descriptors, then it would just take that descriptor and an array of indexes?
< sipa> provoostenator: sure, but i'm talking about right now
< sipa> we can also add descriptor responses to addmultisig etc
< provoostenator> I like the idea of using descriptors for both inputs and outputs wherever feasible.
< instagibbs> basically we're taking advantage of descriptors pre-descriptor-record based wallets
< meshcollider> After I've finished reviewing the last few wallet PRs adding more descriptor support is what I really want to work on anyway
< sipa> that would mean adding descriptor support to signrawtransaction etc
< instagibbs> is there a list of places to be added?
< instagibbs> or is that an action item
< sipa> that's a good idea, we should make such a list
< sipa> i'll open an issue
< provoostenator> Yes please
< meshcollider> +1
< sipa> the question is then: do we go for that, or do we also try to keep the pre-descriptor approach functional?
< sipa> (which needs things like 14481, because listunspent + signrawtransactionwithkey is pretty much broken right now)
< meshcollider> I feel like it would have to be deprecated like accounts were
< sipa> i don't think there is a strong need to actually remove it
< meshcollider> 14481 is only broken for P2SH-P2WSH I think
< sipa> it's more a question of do we keep updating it with new stuff
< meshcollider> Ah yep, well if we're keeping the old stuff it at least needs to be maintained to a working level right
< meshcollider> But not new features no, IMO
< sipa> ok, fair
< ryanofsky_> 14481 also adds a test for signrawtransaction, though that could be added separately
< sipa> ryanofsky_: ah, good
< meshcollider> See also #11708
< gribble> https://github.com/bitcoin/bitcoin/issues/11708 | Add P2SH-P2WSH support to signrawtransaction and listunspent RPC by MeshCollider · Pull Request #11708 · bitcoin/bitcoin · GitHub
< meshcollider> Which was the precursor to 14481
< meshcollider> Which it sounds like is actually a better approach to resurrect after all
< sipa> another topic: #13932
< gribble> https://github.com/bitcoin/bitcoin/issues/13932 | Additional utility RPCs for PSBT by achow101 · Pull Request #13932 · bitcoin/bitcoin · GitHub
< sipa> achow101: when rebase?
< achow101> later today
< sipa> i was thinking about a processpsbtwithdescriptor RPC, and was wondering if it should be integrated with utxoupdatepsbt
< meshcollider> Ok I have to go now, I'll catch up on the rest later today
< sipa> as we have all the pieces in place now
< sipa> or should we keep them separate, where you'd first run utxoupdatepsbt, then processpsbtwithdescriptor to fill in scripts, and then sign (which could be done by any signer, or with processpsbtwithdescriptor at once)
< instagibbs> sipa, would that be a "stateless" call?
< sipa> utxoupdatepsbt is not stateless (it looks at the UTXO set), but processpsbtwithdescriptor would be
< instagibbs> was asking about latter
< instagibbs> so it would involve passing in xprv-containing descriptors when signing required
< sipa> i was thinking you could give it either public descriptors, private descriptors, or private keys separately
< achow101> i think it should stay separate
< sipa> i have a slight preference for that as well, but it's less convenient
< achow101> but do you really need a descriptor to process a psbt? what if we just made it a simple signer?
< achow101> the descriptor is only needed for finalizing
< achow101> i guess you could also fill in keys from a descriptor
< sipa> achow101: to fill in witnessscript/redeemscripts/keys
< sipa> so yes
< provoostenator> Just a key could be a handy shortcut for some descriptors, but perhaps the base case should be a descriptor.
< sipa> provoostenator: just a key is for the case where you have an xpub-based descriptor, and a private key separately
< provoostenator> For the unambiguous case if you have pwpk(blah xpub blah) then if you pass in an xpriv that's clear.
< provoostenator> Right
< provoostenator> But that would just be a shortcut for pwpk(blah xpriv blah)
< sipa> or when the PSBT already has all script fields, and all you have is a private key
< sipa> i'll add them to the issue i'm opening
< sipa> anything else we want to discuss?
< provoostenator> Nope
< sipa> as decreed by provoostenator:
< sipa> #endmeeting
< lightningbot> Meeting ended Fri Nov 16 19:54:56 2018 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< zallarak> meshcollider: I updated per your feedback, and the build is passing. Any other thoughts / comments welcome. I am thinking about a new way of testing, that is why I worked on this.
< zallarak> MarcoFalke: friendly ping if you have any thoughts on the revised PR I submitted (https://github.com/bitcoin/bitcoin/pull/14733)
< bitcoin-git> [bitcoin] sp4ke opened pull request #14740: Update `rpcbind` doc to match the manpage closes #9272 (master...patch-2) https://github.com/bitcoin/bitcoin/pull/14740