< bitcoin-git> [bitcoin] PierreRochard opened pull request #11478: ODB Integration (master...odb) https://github.com/bitcoin/bitcoin/pull/11478
< bitcoin-git> [bitcoin] PierreRochard closed pull request #11478: ODB Integration (master...odb) https://github.com/bitcoin/bitcoin/pull/11478
< bitcoin-git> [bitcoin] kswapd opened pull request #11479: [Docs] Update README.md to add Freenode links (master...master) https://github.com/bitcoin/bitcoin/pull/11479
< bitcoin-git> [bitcoin] tjps opened pull request #11480: [ui] Add toggle for unblinding password fields (master...tjps_wallet_dialog) https://github.com/bitcoin/bitcoin/pull/11480
< bitcoin-git> [bitcoin] AmirAbrams opened pull request #11482: Use CPrivKey typedef for keydata in CKey (master...patch-3) https://github.com/bitcoin/bitcoin/pull/11482
< bitcoin-git> [bitcoin] laanwj pushed 5 new commits to master: https://github.com/bitcoin/bitcoin/compare/5a9da37fb3f4...0e3a41135157
< bitcoin-git> bitcoin/master 3f0ee3e Jorge Timón: Proper indentation for CheckTxInputs and other minor fixes
< bitcoin-git> bitcoin/master 832e074 Jorge Timón: Optimization: Minimize the number of times it is checked that no money is created...
< bitcoin-git> bitcoin/master 3e8c916 Jorge Timón: Introduce CheckInputsAndUpdateCoins static wrapper in txmempool.cpp
< wumpus> meshcollider: listing 'available' wallets would be a lot easier with the walletdir
< bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/0e3a41135157...fef65c4f5e59
< bitcoin-git> bitcoin/master a2be3b6 Jim Posen: [net] Ignore getheaders requests for very old side blocks...
< bitcoin-git> bitcoin/master eff4bd8 Jim Posen: [test] P2P functional test for certain fingerprinting protections
< bitcoin-git> bitcoin/master fef65c4 Wladimir J. van der Laan: Merge #11113: [net] Ignore getheaders requests for very old side blocks...
< bitcoin-git> [bitcoin] laanwj closed pull request #11113: [net] Ignore getheaders requests for very old side blocks (master...net-getheaders-fingerprint) https://github.com/bitcoin/bitcoin/pull/11113
< meshcollider> wumpus: yes indeed, I'm just wondering whether there is any point in adding a list of `available` wallets inside the walletdir to the `listwallets` RPC
< promag> wumpus what is the real use case for that?
< wumpus> promag: for listing available wallets? would be nice in the GUI at least
< wumpus> otherwise peopel have to remember the names
< wumpus> which is okay for the first version or whatever, but in the long run would be nice to select from a list instead
< wumpus> there's no urgency in having it though
< promag> But the app is configured with the wallets, and there is no way to create wallets
< meshcollider> because the only clean-ish way I can think of to find wallets in the walletdir is looking for .dat extensions right?
< meshcollider> promag dynamic loading/unloading of wallets will be coming soon right?
< wumpus> check for berkeleydb databases
< wumpus> (requiring .dat naming is fine with me too, but not all .dats are bdb databases unfortuantely)
< promag> I feel that's not the right path
< promag> not all berkeleydb files are wallets
< wumpus> in the wallet directory they can be assumed to be
< wumpus> but if you want to add an additional check on loading, fine
< wumpus> there's some keys that are only in wallets...
< wumpus> (like the wallet version)
< wumpus> I'm more afraid of users copying valid wallets from other forks though :(
< wumpus> so if we want to add more robustness to wallet loading, yeah, we'd have to consider that...
< meshcollider> The easiest way to check if a file is a bdb file is with magic bytes like ryanofsky suggested here? https://github.com/bitcoin/bitcoin/pull/11466#discussion_r143730935
< wumpus> meshcollider: yes
< wumpus> certainly that's the way to do it without getting berkeleydb involved
< meshcollider> I think I'll take that commit out of my current PR and make a new one
< meshcollider> this will be too much to discuss
< wumpus> but why do you need this now?
< meshcollider> I was going to do the `listwallets` change as part of #11466
< wumpus> I think we need listing available wallets yet, yes lease separate that out
< gribble> https://github.com/bitcoin/bitcoin/issues/11466 | [WIP] Specify custom wallet directory with -walletdir param by MeshCollider · Pull Request #11466 · bitcoin/bitcoin · GitHub
< meshcollider> yep 👍
< promag> I would open the wallet, keep it open, but not load the transactions
< wumpus> I mean I *don't* think we need listing available wallets yet
< promag> listwallets change in a different PR will make your PR faster to review and merge meshcollider
< wumpus> we need to enforce some naming convention on wallets in that case (we don't even require .dat at the moment) as well as make sure wallets are somehow tagged as bitcoin wallet
< wumpus> e.g. by having a key in them that identifies them as being such
< promag> *.wlt :troll:
< meshcollider> Yep its more complicated than i initially thought, seperating it out now
< wumpus> promag: if you want to go that way it's better to go full out and use .bitcoinwallet - to rule out overlap with forks etc
< wumpus> and altcoins
< wumpus> this is a discussion topic when the wallet format is changed
< promag> I was kidding :P but maybe .bitcoin-core-wallet
< wumpus> from berkeleydb to something custom, for example
< wumpus> we certainly don't want to stick with .dat then
< promag> well, I think a good implementation will open the whatever-file-is and use/validate the content
< wumpus> I'm all for being more robust when actually opening wallets
< wumpus> but when scanning for available wallets please don't get bdb involved
< wumpus> if you need to look at magics etc, a quick read-only scan should be enough
< promag> listing an available but invalid wallet is stupid?
< wumpus> but rare!
< promag> ok then
< wumpus> opening with berkeleydb is heavy
< promag> why? does it load everything?
< wumpus> also involves potential writing, you don't want the program to write to files it's not told to touch
< wumpus> yes
< wumpus> it does various things
< promag> ok, I rest
< meshcollider> hah i dont think i can checkout a new branch while building the current one
< meshcollider> ill have to use my tablet instead
< wumpus> meshcollider: you can use git worktrees
< wumpus> and check out different branches in different directories
< meshcollider> ooh really? That's cool, I've never even heard of that being possible before
< meshcollider> I'll look it up
< wumpus> it's awesome, especially with large projects like linux where you really don't want to have multiple full clones
< meshcollider> Yeah :D
< promag> yes, thank god sipa introduced that to me a couple of months ago
< promag> wumpus: easy one #10941, just improves test suite
< wumpus> it also helps for some things to do out-of-tree builds, though mostly to keep you git tree clean, and be able to build for different architectures from one source tree - it doesn't allow you to switch the branch while building
< gribble> https://github.com/bitcoin/bitcoin/issues/10941 | Add blocknotify and walletnotify functional tests by promag · Pull Request #10941 · bitcoin/bitcoin · GitHub
< promag> I think it's time give #11006 a chance?
< gribble> https://github.com/bitcoin/bitcoin/issues/11006 | Improve shutdown process by promag · Pull Request #11006 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] laanwj pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/fef65c4f5e59...364da2c52942
< bitcoin-git> bitcoin/master 9c72a46 John Newbery: [tests] Tidy up forknotify.py
< bitcoin-git> bitcoin/master df18d29 João Barbosa: [tests] Add -blocknotify functional test
< bitcoin-git> bitcoin/master 857b32b João Barbosa: [tests] Add -walletnotify functional test
< bitcoin-git> [bitcoin] laanwj closed pull request #10941: Add blocknotify and walletnotify functional tests (master...2017-07-blocknotify-functional-test) https://github.com/bitcoin/bitcoin/pull/10941
< promag_> wumpus: regarding the eye icon, are there designers contributing? or are the used icons open source?
< wumpus> what's wrong with the eye icon we alread have?
< wumpus> (I edited th4e post)
< promag_> oh there is an eye icon already
< wumpus> if you want a new icon you'd have to find an icon that has the correct license (ideally MIT or public design) or an artist that wants to create such an icon under the appropriate license
< wumpus> jonasschnelli designed most of the current ones
< promag_> this eye? src/qt/res/icons/eye.png
< wumpus> yes :)
< promag_> ok, lgtm
< wumpus> s/public design/public domain
< bitcoin-git> [bitcoin] laanwj closed pull request #11479: [Docs] Update README.md to add Freenode links (master...master) https://github.com/bitcoin/bitcoin/pull/11479
< bitcoin-git> [bitcoin] pedrobranco opened pull request #11483: Fix importmulti bug when importing an already imported key (master...bugfix/fix-importmulti-bug) https://github.com/bitcoin/bitcoin/pull/11483
< bitcoin-git> [bitcoin] pedrobranco opened pull request #11484: Optional update rescan option in importmulti RPC (master...enhancement/optional-update-rescan-on-importmulti) https://github.com/bitcoin/bitcoin/pull/11484
< bitcoin-git> [bitcoin] MeshCollider opened pull request #11485: [WIP] Add `available` field to listwallets RPC (master...201710_listwallets_available) https://github.com/bitcoin/bitcoin/pull/11485
< Chris_Stewart_5> Does anyone have an idea why I wouldn't be able to generate a CPubKey from a CKey after calling k.MakeNewKey(true)
< Chris_Stewart_5> the error: test_bitcoin: key.cpp:153: CPubKey CKey::GetPubKey() const: Assertion `ret' failed.
< Chris_Stewart_5> it seems to be failing in secp256k1
< wumpus> Chris_Stewart_5: that can only fail if the private key is not in the valid range
< Chris_Stewart_5> wumpus: Hmm, is there any reason that calling .MakeNewKey(true) would generate one that is invalid?
< Chris_Stewart_5> because assert(fValid) passes on line 135
< Chris_Stewart_5> line 148*
< wumpus> no, I don't know, it would help to print the generated key I guess
< Chris_Stewart_5> wumpus: wif: KwDiBf89QgGbjEhKnhXJuH7LrciVrZi3qYjgd9M7rFU73Nd2Mcv1
< arubi> it's 0
< arubi> 0000000000000000000000000000000000000000000000000000000000000000 rather
< Chris_Stewart_5> hmmm... so that indicates .MakeNewKey() isn't being called?
< wumpus> indeed, and a private key of zero is not valid, should be at least one
< wumpus> either you must be doing something wrong or there's a bug
< Chris_Stewart_5> Eh, I'm playing with #8469
< gribble> https://github.com/bitcoin/bitcoin/issues/8469 | [POC] Introducing property based testing to Core by Christewart · Pull Request #8469 · bitcoin/bitcoin · GitHub
< Chris_Stewart_5> I'm generating various standard tx types (p2pk,p2pkh,multisig) etc and then running them through the interpreter to assert our interpreter/wallet code base are consistent
< Chris_Stewart_5> When I'm generating a std::vector<CKey> this bug appears, but does not appear when I generate a single CKey
< Chris_Stewart_5> arubi: Thanks by the way
< arubi> oh cheers
< wumpus> right, next step would be to figure out why it returns the zero key. I'm happy secp256k1 catches this at least.
< Chris_Stewart_5> wumpus: Agreed! It is weird because if I try to explicitly call .MakeNewKey(true) (again) before I convert it to a pubkey secp256k1 gives this error
< wumpus> yes I was still in the process of trying to convert that key to raw bytes - a google would have been faster in this case :)
< Chris_Stewart_5> [libsecp256k1] illegal argument: seckey != NULL
< Chris_Stewart_5> perhaps my generator is generating a valid 0 byte key, which is *technically* a valid key right?
< Chris_Stewart_5> and secp256k1 just says 'you really don't want to be doing this...'
< wumpus> not sure about "technically" in this case - I think it's mathemtically undefined
< arubi> it's not a valid key in secp256k1, but it's base58 encoding is valid
< wumpus> right ^^
< arubi> so something generated it..
< wumpus> so are you expecting a random key?
< Chris_Stewart_5> wumpus: Yes... here is the sequence I am using to generate one: https://github.com/Christewart/bitcoin/blob/af2f6f2a639d94945a73e63f9f4203071caad984/src/test/gen/crypto_gen.h#L18
< Chris_Stewart_5> do you see something inherently wrong with it?
< jnewbery> meshcollider: sorry - missed the discussion earlier. I think updating listwallets to list available wallets would definitely be a useful feature, but it's by no means necessary for #11466.
< gribble> https://github.com/bitcoin/bitcoin/issues/11466 | Specify custom wallet directory with -walletdir param by MeshCollider · Pull Request #11466 · bitcoin/bitcoin · GitHub
< jnewbery> promag: Both your alternative schemas in https://github.com/bitcoin/bitcoin/pull/11485#issuecomment-335813034 are fine. I'm not too concerned about the exact format, but I think it's useful for users to be able to access the information somehow
< Chris_Stewart_5> wumpus: For what it is worth it is unrelated to anyting in core. I wasn't capturing a variable correctly in a lambda
< promag> Right, I just think there is no strong use case for that
< promag> I kind of like findwallets because it can return "invalid" wallets where as listwallets is returning valid and loaded wallets
< promag> at the moment listwallet can return available "invalid" wallet that cannot be loaded
< promag> btw, do you think we should have the inverse of -experimentalrpc=?
< promag> so 0.15 introduced listwallets but to use it it should be enabled like so
< promag> sorry, **the inverse of -deprecaterpc**
< promag> therefore all experimental rpc can change
< jnewbery> The use case is being able to find and load wallets dynamically at run-time. If you have any concept feedback on that, the PR is 10740
< jnewbery> re: -experimentalrpc - I don't think that's necessary. Release notes for multiwallet stated 'Note that while multi-wallet is now fully supported, the RPC multi-wallet interface should be considered unstable for version 0.15.0, and there may backwards-incompatible changes in future versions.
< promag> ok and ok
< promag> wumpus: you deleted the comment in #11476?
< gribble> https://github.com/bitcoin/bitcoin/issues/11476 | Avoid opening copied wallet databases simultaneously by ryanofsky · Pull Request #11476 · bitcoin/bitcoin · GitHub
< wumpus> no?
< timothy> little-OT: do you think bitcoin gold have enough time to implement replay protection?
< timothy> if no, it may be a big problem for (some) bitcoin users too due to replay attack
< jnewbery> timothy: #bitcoin please. This channel is for discussing Bitcoin Core development
< bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/364da2c52942...892809309c1b
< bitcoin-git> bitcoin/master 619bb05 MarcoFalke: Squashed 'src/univalue/' changes from 16a1f7f6e..fe805ea74...
< bitcoin-git> bitcoin/master faaeeb0 MarcoFalke: Bump univalue and fix json formatting in tests...
< bitcoin-git> bitcoin/master 8928093 Wladimir J. van der Laan: Merge #11420: Bump univalue subtree and fix json formatting in tests...
< wumpus> if you get any linker errors while building master, you probably need to clean your tree after #11420 (it seems that some changes to univalue are not detected by the build system)
< gribble> https://github.com/bitcoin/bitcoin/issues/11420 | Bump univalue subtree and fix json formatting in tests by MarcoFalke · Pull Request #11420 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] laanwj closed pull request #11420: Bump univalue subtree and fix json formatting in tests (master...Mf1709-bumpUnivalue) https://github.com/bitcoin/bitcoin/pull/11420
< bitcoin-git> [bitcoin] laanwj closed pull request #11445: [qa] 0.15.1 Backports (0.15...Mf1710-0151qaBackports) https://github.com/bitcoin/bitcoin/pull/11445
< Chris_Stewart_5> Does ProduceSignature have the capability to sign a p2sh(multisig) tx? Here is how I am trying to use it https://gist.github.com/Christewart/3d930327e7b27e6a897fa6d2744ec569
< Chris_Stewart_5> if not, is there capability for this else where in the code base?
< sipa> Chris_Stewart_5: yes, it does
< sipa> but you need to have the redeemscript in the keystore
< Chris_Stewart_5> sipa: ^
< sipa> your redeemscript is just a key?
< sipa> or what is spk_keys
< Chris_Stewart_5> tuple
< Chris_Stewart_5> std::tuple<CScript,std::vector<CKey>>
< sipa> and what's the script?
< Chris_Stewart_5> one of the following: P2PK CScript, P2PKH CScript, Multisig CScript
< Chris_Stewart_5> It is randomly choosing one of those
< Chris_Stewart_5> then creating a p2sh script
< sipa> looks right to me
< sipa> what is the problem?
< Chris_Stewart_5> the assert fails on this line -- it should return complete right? https://gist.github.com/Christewart/3d930327e7b27e6a897fa6d2744ec569#file-produce_signature_p2sh-cpp-L20
< Chris_Stewart_5> *theoretically* haha
< sipa> yes
< sipa> i don't see anything obviously wrong
< Chris_Stewart_5> I'll dig more -- it is probably something stupid I am doing else where. I just wanted to confirm produce signature has the capability before I go deeper
< morcos> I'm just trying to catch up on old review by looking at what was merged.. In #11113 , what is the use case of responding to these strange getheaders requests with no locator at all? Or at the very least shouldn't we only respond if the hashStop is on our main chain
< gribble> https://github.com/bitcoin/bitcoin/issues/11113 | [net] Ignore getheaders requests for very old side blocks by jimpo · Pull Request #11113 · bitcoin/bitcoin · GitHub
< morcos> Not objection to merging the PR, it seems like a strict improvement, but just trying to understand why that functionality even exists
< bitcoin-git> [bitcoin] mess110 opened pull request #11486: [tests] Add uacomment tests (master...test_uacomment) https://github.com/bitcoin/bitcoin/pull/11486
< bitcoin-git> [bitcoin] TheBlueMatt opened pull request #11487: Check that new headers are not a descendant of an invalid block (master...2017-10-acceptblock-validity-check) https://github.com/bitcoin/bitcoin/pull/11487
< bitcoin-git> [bitcoin] C0deAi opened pull request #11488: Codeai fixes: remove dead code, prevent possible division by zero. (master...codeai-fixes) https://github.com/bitcoin/bitcoin/pull/11488