< 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, 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/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
< 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
< 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] 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] 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