< bitcoin-git> [bitcoin] fanquake closed pull request #15052: Tests: Contract testing for the procedure AddTimeData and related fixes (master...timedata_contract_test2) https://github.com/bitcoin/bitcoin/pull/15052
< bitcoin-git> [bitcoin] fanquake closed pull request #15104: Tests: Add unit testing for the CompressScript function (master...compress_contract_test3) https://github.com/bitcoin/bitcoin/pull/15104
< bitcoin-git> [bitcoin] fanquake closed pull request #13357: Define SIGHASH_MASK in validation and determine the use of SIGHASH_SINGLE in signing (master...signsingle) https://github.com/bitcoin/bitcoin/pull/13357
< phantomcircuit> sipa, i added logging of the leveldb wrapper stuff and noticed that there's requests for seemingly everything until block height=227931 where the cache seems to absorb all the requests, does that make any sense or have i just missed something really obvious
< sipa> phantomcircuit: elaborate?
< phantomcircuit> sipa, i added some LogPrint statements in dbwrapper.h after pdb->Get in Read and Exists
< phantomcircuit> i wanted to do a graph of "io" relative to dbcache size during IBD
< phantomcircuit> but instead what im seeing in logs seems crazy
< phantomcircuit> sipa, i changed the logging hang on i'll upload the debug.log...
< phantomcircuit> sipa, https://2083236893.com/debug.log
< phantomcircuit> sipa, i think that's the block where bip34 was activated
< harding> phantomcircuit: BIP90 agrees that 227931 was the BIP34 activationheight.
< sipa> phantomcircuit: will look tomorrow
< sipa> threre is some interaction between bip34 and the utxo logic
< phantomcircuit> oh i see bip30
< phantomcircuit> sipa, nvm i understand how, the bip30 logic guarantees at least one utxo db access for each transaction in the block until the bip34 activation block
< phantomcircuit> and since it's never going to be in the cache the read always goes to disk
< phantomcircuit> there's probably potential to optimize that when assumevalid is set
< phantomcircuit> it's 34 million leveldb Get calls
< bitcoin-git> [bitcoin] sipsorcery opened pull request #16483: Updated python command in msvc readme (master...update_msvc_readme) https://github.com/bitcoin/bitcoin/pull/16483
< bitcoin-git> [bitcoin] fanquake closed pull request #16458: Fix msvc compiler error C4146 (unary minus operator applied to unsigned type (0.17...fix-C4146-in-util-test) https://github.com/bitcoin/bitcoin/pull/16458
< jonasschnelli> hebasto: https://github.com/bitcoin/bitcoin/pull/16476,... 5 labels? :-)
< jonasschnelli> Draht added them... I see
< fanquake> jonasschnelli: sometimes the bot gets a bit out of control hah
< jonasschnelli> sipa: the PR #16202 is a result of your NACK/comment in 14046. Appreciate your review. Thanks
< gribble> https://github.com/bitcoin/bitcoin/issues/16202 | Refactor network message deserialization by jonasschnelli · Pull Request #16202 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] MarcoFalke reopened pull request #16458: Fix msvc compiler error C4146 (unary minus operator applied to unsigned type (0.17...fix-C4146-in-util-test) https://github.com/bitcoin/bitcoin/pull/16458
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #16458: Fix msvc compiler error C4146 (unary minus operator applied to unsigned type (0.17...fix-C4146-in-util-test) https://github.com/bitcoin/bitcoin/pull/16458
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/5c2885f9b2e3...502ec0227290
< bitcoin-git> bitcoin/master fabfcb5 MarcoFalke: build: Treat -Wswitch as error when --enable-werror
< bitcoin-git> bitcoin/master 502ec02 MarcoFalke: Merge #16424: build: Treat -Wswitch as error when --enable-werror
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #16424: build: Treat -Wswitch as error when --enable-werror (master...1907-buildSwitchError) https://github.com/bitcoin/bitcoin/pull/16424
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #16484: doc: Remove "trivial" label in CONTRIBUTING (master...1907-docNoTrivial) https://github.com/bitcoin/bitcoin/pull/16484
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/502ec0227290...f735851be294
< bitcoin-git> bitcoin/master 07e01d6 Jon Atack: rpc: sendrawtransaction unconditionality/privacy note
< bitcoin-git> bitcoin/master f735851 Wladimir J. van der Laan: Merge #16467: rpc: sendrawtransaction help privacy note
< bitcoin-git> [bitcoin] laanwj merged pull request #16467: rpc: sendrawtransaction help privacy note (master...sendrawtransaction-privacy-note) https://github.com/bitcoin/bitcoin/pull/16467
< bitcoin-git> [bitcoin] laanwj pushed 5 commits to master: https://github.com/bitcoin/bitcoin/compare/f735851be294...29220250c10e
< bitcoin-git> bitcoin/master 98a64bd fanquake: build: disable libjpeg in qt
< bitcoin-git> bitcoin/master 0aeb98a fanquake: build: remove jpeg lib check from bitcoin_qt.m4
< bitcoin-git> bitcoin/master 1bb1661 fanquake: doc: fix typo in bitcoin_qt.m4 comment
< bitcoin-git> [bitcoin] laanwj merged pull request #16441: build: remove qt libjpeg check from bitcoin_qt.m4 (master...remove-qt-libjpeg-check) https://github.com/bitcoin/bitcoin/pull/16441
< bitcoin-git> [bitcoin] MarcoFalke pushed 5 commits to master: https://github.com/bitcoin/bitcoin/compare/29220250c10e...74ea1f3b0f26
< bitcoin-git> bitcoin/master 3199610 Fabian Jahr: Place out args at the end for CreateWallet
< bitcoin-git> bitcoin/master d6649d1 Fabian Jahr: Use strong enum for WalletCreationStatus
< bitcoin-git> bitcoin/master ba1f128 Fabian Jahr: Return error for ignored passphrase through disable private keys option
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #16399: wallet: Improve wallet creation (master...followup-16244) https://github.com/bitcoin/bitcoin/pull/16399
< bitcoin-git> [bitcoin] laanwj pushed 6 commits to master: https://github.com/bitcoin/bitcoin/compare/74ea1f3b0f26...b21acab82fe9
< bitcoin-git> bitcoin/master 9f76e45 Hennadii Stepanov: Drop support of insecure miniUPnPc versions
< bitcoin-git> bitcoin/master 91a1b85 Hennadii Stepanov: Use PACKAGE_NAME in UPnP description
< bitcoin-git> bitcoin/master 02709e9 Hennadii Stepanov: Align formatting with clang-format
< bitcoin-git> [bitcoin] laanwj merged pull request #15993: net: Drop support of the insecure miniUPnPc versions (master...20190506-drop-ancient-miniupnpc-api) https://github.com/bitcoin/bitcoin/pull/15993
< sdaftuar> hi all, if anyone is interested in reviewing or even just testing #15759 i'd appreciate it, as i think it'd be best if p2p changes like this simmer in master with plenty of time before a new release in case of bugs or unintended side effects
< gribble> https://github.com/bitcoin/bitcoin/issues/15759 | [p2p] Add 2 outbound blocks-only connections by sdaftuar · Pull Request #15759 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/b21acab82fe9...2a7c3bc498b5
< bitcoin-git> bitcoin/master 4057b7a Hennadii Stepanov: wallet: Recognize -disablewallet option early
< bitcoin-git> bitcoin/master 2a7c3bc Wladimir J. van der Laan: Merge #16436: gui: Do not create payment server if -disablewallet option p...
< wumpus> sdaftuar: agreed
< bitcoin-git> [bitcoin] laanwj merged pull request #16436: gui: Do not create payment server if -disablewallet option provided (master...20190722-payment-server) https://github.com/bitcoin/bitcoin/pull/16436
< sdaftuar> sipa: wumpus: any thoughts on the issue described in #16444? seems like an annoying problem to fix
< gribble> https://github.com/bitcoin/bitcoin/issues/16444 | Assertion `setBlockIndexCandidates.count(pindex) failed · Issue #16444 · bitcoin/bitcoin · GitHub
< sdaftuar> i think if we want to maintain the invariants we've historically had and not change CheckBlockIndex() at all, then we could add some more code to InvalidateBlock() so that everything works as it used to.
< sdaftuar> it just seems a bit tedious and it's not totally clear to me that it's important to maintain our usual invariants in this one specific case
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/2a7c3bc498b5...68da54987df4
< bitcoin-git> bitcoin/master 42a5e91 John Newbery: [mempool] log correct messages when CPFP fails
< bitcoin-git> bitcoin/master 68da549 Wladimir J. van der Laan: Merge #16471: [mempool] log correct messages when CPFP fails
< bitcoin-git> [bitcoin] laanwj merged pull request #16471: [mempool] log correct messages when CPFP fails (master...2019-07-fix-CalculateMempoolAncestors-logging) https://github.com/bitcoin/bitcoin/pull/16471
< bitcoin-git> [bitcoin] pstratem opened pull request #16486: [consensus] skip bip30 checks when assumevalid is set for the block (master...2019-07-29-fassumevalid-bip34) https://github.com/bitcoin/bitcoin/pull/16486
< bitcoin-git> [bitcoin] jamesob opened pull request #16487: validationinterface: add unused CChainState parameter (master...2019-07-au-vi-arg) https://github.com/bitcoin/bitcoin/pull/16487
< phantomcircuit> also the TODO in the BIP30 logic while not exactly immediate should be looked into
< bitcoin-git> [bitcoin] jonatack opened pull request #16489: log: update bitcoind daemon logging (master...daemon-logging-harmonisation) https://github.com/bitcoin/bitcoin/pull/16489
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #16490: rpc: Report reason for 'bip125-replaceable' value (master...1907-rpcMempoolWhyReplacable) https://github.com/bitcoin/bitcoin/pull/16490
< bitcoin-git> [bitcoin] jonatack opened pull request #16491: qa: fix deprecated log.warn in feature_dbcrash test (master...test-fix-feature_dbcrash-warn-deprecation) https://github.com/bitcoin/bitcoin/pull/16491
< sipa> jonasschnelli, BlueMatt: about v2 transport and the question on how to encode the message types. given that 12 bytes is complete overkill anyway, i wonder if an alternative to shortcuts+variable length as proposed currently, it wouldn't be easier to just shrink the message type field; it turns out that first 2 characters + last 2 characters of every message name is unique currently
< BlueMatt> why?
< BlueMatt> just use the current one-byte proposal with a fallback to 12 bytes?
< BlueMatt> or fallback to, I dunno, 8 bytes? whatever you want
< sipa> that's also a possibility, but harder to extend
< BlueMatt> is it?
< sipa> as in: the shortening will only be available to a select number of messages, and probably need a lot of coordination to avoid collisions between them
< BlueMatt> a lot of coordination? as in te bip process?
< sipa> right
< BlueMatt> that seems reasonable to me.
< sipa> string messages are unlikely to collide, for 1-byte types you really need to assign sequentially
< BlueMatt> how about 8 bytes and you must use the time of when you selected the message :p
< sipa> while 4-byte messages for everything doesn't have this problem, is even simpler to implement, gets you most of the savings, and is backward compatible
< BlueMatt> ok, or 4 byte message types and you must use the time of when you selected the message mod 2**32 :p
< sipa> that could totally work
< BlueMatt> I prefer that infinitely over a variable-length message type anyway
< sipa> infinity is a lot
< BlueMatt> YUGE
< sipa> filterclear would become "feer".
< BlueMatt> limiting to common-char-lower-case-ascii seems like a waste of 4 bytes, but, whatever
< sipa> it is; for v2-only messages that restriction wouldn't be needed
< sipa> actually it's need needed at all, agree
< sipa> or we could use SHA256(message)[0:4] :p
< sipa> oh no, double-SHA256(message)[0:4], satoshi style
< BlueMatt> rot13(rot13(message))
< sipa> i'm only half joking; using a hash function or CRC or so as mapping from string names to 4-byte message type ids has hardly any downsides i think
< BlueMatt> true
< sipa> an actual implementation wouldn't actually compute the hashes; they'd just be precomputed for every message type
< BlueMatt> sure, of course
< sipa> anyway, let's see what jonasschnelli thinks
< hugohn> sipa: do you think it makes sense for descriptor.cpp->Parse() to return the output type of the descriptor string, along with the Signing Provider? I'm trying to infer the output type through other means without touching descriptor.h/.cpp. But Expand() doesn't work as intermediate redeem scripts are no longer saved to memory with native descriptor wallets (so you can’t peek into them). And string pattern matching
< hugohn> (e.g. matching against `"sh(wsh"`) is ugly IMO.
< sipa> hugohn: what is 'output type' ?
< sipa> and i don't understand why Expand doesn't work
< sipa> oh, output type as in address type
< sipa> hmm
< sipa> what about raw multisig, or combo? those don't have a well-defined output type
< hugohn> the "buckets" used in the refactored wallet
< sipa> Expand() + ExtractDestination should work, i think
< sipa> and you should always be able to call Expand on descriptors in the wallet, otherwise how would you participate in signing?
< sipa> and you don't need the intermediate redeemscript; just ExtractDestination on the output scriptPubKey should be sufficient
< sipa> i may be missing things :)
< hugohn> yeah I can call Expand just fine :D my problem is distinguishing between a legacy P2SH & a P2SH-Segwit
< sipa> there shouldn't be a need to distinguish between those
< sipa> if the sender supports P2SH, he will support both
< sipa> as he obviously can't distinguish
< sipa> output types currently distinguish between them, as that what the mechanism to determine whether you wanted to use segwit in the wallet or not
< sipa> but in a post-descriptor world, i think that distinction would simply be made by having a segwit descriptor or not
< hugohn> right, it doesn't make a difference to the user, but I believe in achow101 's proposed new wallet architecture, we would have different scriptPubKeyManager(s) for each of the 3 output types: LEGACY, P2SH_SEGWIT (wrapped Segwit), and BECH32.
< sipa> right, and you need a sanity check when creating a new descriptor?
< sipa> that the descriptor is compatible with that type
< sipa> ?
< achow101> Yes
< sipa> arguably we should just get rid of the distinction between P2SH and P2SH-Segwit
< achow101> I wrote a function that used the Solver + a SigningProvider in order to determine the address type, but apparently that doesn't work since the p2sh-segwit redeemScript is not put in the SigningProvider by Descriptor Expand or Parse
< sipa> though perhaps that's not something to do simultaneously
< sipa> hmm, really?
< sipa> Expand should put it there
< achow101> That's what I thought, but hugohn told me it isn't (haven't had the chance to check)
< sipa> that sounds like a major issue if it isn't
< sipa> hugohn: yes?
< hugohn> sorry ignore that snippet. I missed that line you posted above, looks like the redeem script is saved to the Signing Provider.
< sipa> hugohn: in general no information should get lost through an Expand
< hugohn> then I don't understand why when we look up for the redeemScript later, we don't find it, hmm...
< sipa> otherwise InferDescriptor wouldn't be able to reconstruct things
< sipa> which afaik has a test
< elichai2> hugohn: maybe it wasn't expanded yet?
< hugohn> elichai2: no I made sure it was expanded
< hugohn> I'm not getting the correct output type by peeking into an address generated by Expand() here^
< hugohn> `provider.GetCScript()` fails to look up the redeem script given the script ID
< hugohn> nway, very likely I'm doing something wrong, will investigate further. thanks sipa!
< sipa> hugohn: add some debug statements that print the contents of the SigningProvider before calling the code to determine output type
< hugohn> sipa: yes I'm adding printf eveywhere lol. will try that next!