< bitcoin-git> [bitcoin] Sjors opened pull request #16257: [wallet] abort when attempting to fund a transaction above -maxtxfee (master...2019/06/max_fee) https://github.com/bitcoin/bitcoin/pull/16257
< promag> achow101: have you tested #16215?
< gribble> https://github.com/bitcoin/bitcoin/issues/16215 | gui: Refactor wallet controller activities by promag · Pull Request #16215 · bitcoin/bitcoin · GitHub
< promag> I can split, no problem
< promag> achow101: actually if I split then I'd have to implement CreateWalletActivity, like current OpenWalletActivity, which is not that great compared to 16215.
< promag> ryanofsky suggestion makes sense because avoids refactor PRs, but if I split then we would have different refactor: refactor both activities to a better approach.
< promag> what makes more sense to me is to first refactor OpenWalletActivity, then your PR could have my the remaining changes squashed in the correct commits.
< promag> let me know what you think
< promag> fanquake: mind testing #16231? ty
< gribble> https://github.com/bitcoin/bitcoin/issues/16231 | gui: Fix open wallet menu initialization order by promag · Pull Request #16231 · bitcoin/bitcoin · GitHub
< fanquake> promag ok
< jonasschnelli> Is is somehow possible to emulate BE with Qemu? With reasonable performance? I remember doing it years ago but it was veeery slow?
< jonasschnelli> Or is it possible to use an ARM SoC-board (Odroid) to run a BE OS on ARM64?
< kallewoof> jonasschnelli: yes
< jonasschnelli> kallewoof: can you be more specific. :)
< kallewoof> I have a qemu instance for trying bitcoin compiling
< kallewoof> big endian.
< kallewoof> its not reasonable performance though, so... maybe "sorta" is more accurate than "yes".
< jonasschnelli> kallewoof: okay. But the compilation finishes at "some point"? Can you run the unit tests?
< kallewoof> yep!
< jonasschnelli> Great. Do you use PPC? Do you use Debian (since I guess they sill support PPC images)?
< kallewoof> its debian_wheezy_powerpc_standard.qcow2
< kallewoof> unfortunately debian stopped supporting ppc after wheezy
< jonasschnelli> Looks like everyone is using that
< jonasschnelli> The question is, if BE is reasonable anymore...
< kallewoof> i thought there was new hardware that was BE or could be BE
< jonasschnelli> BE is probably relevant fpr the Power9 family
< bitcoin-git> [bitcoin] NicolasDorier opened pull request #16258: [MSVC]: Create the config.ini as part of bitcoind build (master...msvc/build-config-ini) https://github.com/bitcoin/bitcoin/pull/16258
< kallewoof> @sipa I am confused about a commit of yours from March 2016 (sorry..!), but someone else might be able to explain this: https://github.com/bitcoin/bitcoin/commit/8a253b342c5272496926ed391b078742bbe937ae -- the nInnerLoopCount restricts the nonce to 0..65535 (16 bits), but the nonce is 32 bits. Why? I realize if you make this 0xffffffff and try to Ctrl-C a bitcoind instance while trying to find a blokc, it will lock up and
< kallewoof> wait for the generate to finish. Is that why?
< bitcoin-git> [bitcoin] meshcollider pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/23815ee74dfd...303ec103bacf
< bitcoin-git> bitcoin/master a495034 Andrew Chow: Make and get the multisig redeemscript and destination in one function ins...
< bitcoin-git> bitcoin/master 303ec10 MeshCollider: Merge #16026: Ensure that uncompressed public keys in a multisig always re...
< bitcoin-git> [bitcoin] meshcollider merged pull request #16026: Ensure that uncompressed public keys in a multisig always returns a legacy address (master...dont-sw-ms-uncomp) https://github.com/bitcoin/bitcoin/pull/16026
< bitcoin-git> [bitcoin] meshcollider pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/303ec103bacf...fd333e15a545
< bitcoin-git> bitcoin/master 7c611e2 Andrew Chow: Move ismine to wallet module
< bitcoin-git> bitcoin/master e61de63 Andrew Chow: Change ismine to take a CWallet instead of CKeyStore
< bitcoin-git> bitcoin/master fd333e1 MeshCollider: Merge #16226: Move ismine to the wallet module
< bitcoin-git> [bitcoin] meshcollider merged pull request #16226: Move ismine to the wallet module (master...mv-ismine-wallet) https://github.com/bitcoin/bitcoin/pull/16226
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/fd333e15a545...32e94538185b
< bitcoin-git> bitcoin/master 5a88ea7 Jon Atack: doc: remove orphaned header in developer notes
< bitcoin-git> bitcoin/master 32e9453 fanquake: Merge #16256: doc: remove orphaned header in developer notes
< bitcoin-git> [bitcoin] fanquake merged pull request #16256: doc: remove orphaned header in developer notes (master...remove-orphaned-header-link-in-developer-notes) https://github.com/bitcoin/bitcoin/pull/16256
< bitcoin-git> [bitcoin] natangl opened pull request #16260: Yehuda (master...yehuda) https://github.com/bitcoin/bitcoin/pull/16260
< bitcoin-git> [bitcoin] fanquake closed pull request #16260: Yehuda (master...yehuda) https://github.com/bitcoin/bitcoin/pull/16260
< MarcoFalke> #proposedmeetingtopic 0.18.1: Backports #16035
< gribble> https://github.com/bitcoin/bitcoin/issues/16035 | 0.18.1: Backports by MarcoFalke · Pull Request #16035 · bitcoin/bitcoin · GitHub
< phantomcircuit> kallewoof, there's a pr with comments https://github.com/bitcoin/bitcoin/pull/7663
< kallewoof> @phantomcircuit thanks!
< phantomcircuit> kallewoof, you can get to those from the commit btw
< kallewoof> how?
< kallewoof> OH.
< kallewoof> doh. thanks.
< phantomcircuit> it's right above the author
< phantomcircuit> yeah i only noticed that recently
< bitcoin-git> [bitcoin] promag opened pull request #16261: gui: Refactor OpenWalletActivity (master...2019-06-refactor-open-wallet) https://github.com/bitcoin/bitcoin/pull/16261
< kallewoof> phantomcircuit: unfortunately the PR doesnt give me any more clues as to why it only uses 16 out of 32 bits of the nonce...
< phantomcircuit> kallewoof, well it's just the inner loop
< phantomcircuit> yeah actually not sure about that one
< kallewoof> I think the idea was to make it run 65535 times and then loop back and then run another 65535 times and then if it ran out of the nonce it would inccrement the extra nonce, but thats not what its doing at all.
< phantomcircuit> kallewoof, i mean im sure it doesn't matter at least
< kallewoof> why not?
< phantomcircuit> kallewoof, the inner loop is doing all of the hashing for the block again anyways
< phantomcircuit> so changing the extra nonce value even every loop wouldn't make it any slower
< kallewoof> you lose 16 bits worth of nonce space though
< phantomcircuit> so what
< phantomcircuit> that's not even a vaguely efficient loop regardless
< kallewoof> it doesnt matter for regtest, or other trivially mineable nets, no. i am having issues with signet though cause i have to actually re-create signatures every time you run out of nonces. having 32 bits intead of 16 would make things a lot less grumpifying.
< sipa> kallewoof: the code for the generate RPC is designed for simplicity, not efficiency
< kallewoof> sipa: yeah, just wondering if that 16 bit cap was for a reason or if it was just arbitrary
< phantomcircuit> sipa, wait wouldn't that be a hard stop at the inner loop limit instead of at the nmaxtries?
< phantomcircuit> actually nvm im dumb
< phantomcircuit> it should be a uint32_t though
< * phantomcircuit> runs
< phantomcircuit> kallewoof, i cant measure a performance difference between the loop stopping at 16 vs 32 bits searched
< phantomcircuit> takes about 1 minute to generate 1000 blocks on a new regtest chain either way
< phantomcircuit> (sorry 10000)
< kallewoof> phantomcircuit: thats not the point. in signets case i have to actually ask every signer to resign whenever we run out of nonce space. i can use custom mining software but that seems overkill for a test network
< kallewoof> 65535 combos vs 4294967296 makes a big difference when you have to generate signatures and stuff. esp if you are doing multi-sig with peers all over the place.
< phantomcircuit> kallewoof, i don't see any reason not to make the search space 32 bits
< kallewoof> great! me neither. :)
< achow101> meshcollider: do you think #15588 is mergable? It's a pretty simple change
< gribble> https://github.com/bitcoin/bitcoin/issues/15588 | Log the actual wallet file version and no longer publicly expose the "version" record by achow101 · Pull Request #15588 · bitcoin/bitcoin · GitHub
< phantomcircuit> actually i dont see any reason for the inner loop check at all
< phantomcircuit> kallewoof, actually i see why, the ShutdownRequested() check in the loop
< phantomcircuit> only gets called once per search of the nonce space
< kallewoof> Yeah, that seems to be the only reason.
< kallewoof> And thats what I noticed when I upped it to 0xffffffff.. I couldnt actually ^C without waiting for the entire 32 bits to iterate through.
< phantomcircuit> on x86 it should be cheap enough to check that every inner loop
< sipa> this was before we had atomic variables
< phantomcircuit> sipa, yeah no criticism from me there
< phantomcircuit> makes perfect sense this is how you did it
< bitcoin-git> [bitcoin] pstratem opened pull request #16262: Slightly simplify the mining loop. (master...2019-06-21-generateblocks) https://github.com/bitcoin/bitcoin/pull/16262
< phantomcircuit> i help
< kallewoof> i did something more complex. your solution is better.
< promag> Can I have #16261 in HP?
< gribble> https://github.com/bitcoin/bitcoin/issues/16261 | gui: Refactor OpenWalletActivity by promag · Pull Request #16261 · bitcoin/bitcoin · GitHub
< promag> achow101: FYI, if you rebase with that you'll have a couple of trivial conflicts
< bitcoin-git> [bitcoin] hebasto opened pull request #16263: qt: Use qInfo() if no error occurs (master...20190621-qInfo) https://github.com/bitcoin/bitcoin/pull/16263
< sipa> if there is a wallet meeting today, i'll miss it (meeting with github)
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #16264: policy: Add experimental -mempoolreplacement=full (off by default) (master...1906-policyFullRbf) https://github.com/bitcoin/bitcoin/pull/16264
< achow101> wallet meeting?
< meshcollider> #startmeeting
< lightningbot> Meeting started Fri Jun 21 19:00:21 2019 UTC. The chair is meshcollider. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< meshcollider> #bitcoin-core-dev Wallet Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator aj Chris_Stewart_5 dongcarl gwillen jamesob ken281221 ryanofsky
< kanzure> hi
< achow101> hi
< achow101> something I mentioned yesterday that we could discuss here: "I've been working on implementing the scriptpubkeymanager and I noticed a bunch of things related to key generation rely on the wallet version and wallet flags. Thoughts on how to handle those without introducing a circular dependency?"
< achow101> sipa's suggestion was to have flags and version be part of the constructors for SPKmanagers, but these things get updated during key generation and imports
< achow101> I tried having the spkmanager write out the updates to the wallet file, but then the parent cwallet instance wouldn't know about the flag changes. it also felt like a layer violation
< achow101> any thoughts on that?
< meshcollider> #topic scriptpubkeymanager wallet flags (achow101)
< kanzure> managers are assigned a key, or managers make keys?
< achow101> managers make keys
< meshcollider> Can the wallet pass a pointer on construction instead?
< achow101> yes, but locks..
< achow101> IIRC cs_wallet needs to be locked to modify flags but spkmanagers won't have access to cs_wallet
< achow101> FWIW all of the wallet flags are things that spkmanagers care about, not the wallet. so maybe it does make sense to just have the wallet's flag stuff call through to the spkmanager's?
< meshcollider> Hmm, pass a callback function down from the wallet then?
< instagibbs> which things/flags get updated on import, just to job my memory
< meshcollider> Which SPKManager would provide the flags then
< achow101> instagibbs: WALLET_FLAG_BLANK_WALLET changes on import and key generation. WALLET_FLAG_KEY_ORIGIN_METADATA can change on load. WALLET_FLAG_DISABLE_PRIVATE_KEYS is checked before keys are generated
< achow101> meshcollider: I think they would all have the same flags, so it doesn't matter. but I guess there's an update problem with that too..
< achow101> the only flag that SPKManagers won't use is WALLET_FLAG_AVOID_REUSE
< achow101> my initial thought was to have each spkmanager also store a pointer to it's parent cwallet, but that's circular
< meshcollider> I guess BLANK_ and KEY_ORIGIN_ can be per-spkmanager and the wallet can itetate over them all with OR
< meshcollider> Iterate*
< meshcollider> Whereas DISABLE_PRIV doesn't change does it
< achow101> it doesn't
< meshcollider> Am I right in thinking these flags only ever get set, never reset
< meshcollider> So you can just OR them all
< achow101> yeah
< meshcollider> bitwise
< achow101> so should there be a new wallet record for spkmanager flags?
< achow101> I don't think that approach will really work though since there won't necessarily be a way to uniquely identify spkmanagers
< achow101> the whole loading part is kind of an issue
< meshcollider> Just because one SPKManager changes a flag doesn't mean they all need to update their flags do they? It doesn't matter if they all get the same flags at reload?
< meshcollider> As long as whichever one updates can write it to the wallet file
< achow101> I think so?
< achow101> so i guess the conclusion is that spkmanagers will handle the storage of BLANK, KEY_ORIGIN, an DISABLE_PRIV flags and CWallet handles the storage of AVOID_REUSE
< meshcollider> seems sensible
< achow101> i can foresee some concurrency issues with this
< meshcollider> Hmm I don't really see why there needs to be concurrency issues if they're not being used to communicate with each other at runtime
< meshcollider> Are there any other topics or is this the only one? Just so we aren't holding up the meeting if we continue discussing this
< meshcollider> Doesn't seem like attendance is very high today so I assume this is it :)
< achow101> the flags between cwallet and each spkmanager can be out of sync, so the update of flags on one object can undo an update from another one
< meshcollider> But like I said earlier, isn't each flag only one-way
< achow101> e.g. cwallet has BLANK set. spkmanager unsets BLANK because a seed is set so it writes that to the file. CWallet still has BLANK set. it updates the flags because AVOID_REUSE is changed so now the old BLANK is written back to the wallet
< achow101> not so much concurrency than not updating objects i suppose
< meshcollider> CWallet wouldn't store BLANK anyway
< meshcollider> If it needs to know blank, it would ask the spkmanagers
< achow101> yeah, but the wallet flags record is just a bitfield. it can't update it without updating the whole thing
< achow101> so it needs to know all flags in effect
< achow101> maybe function callbacks are the best option
< meshcollider> It seems like it might be, or can you use the built in atomic stuff to avoid locks
< achow101> i'll try it out
< meshcollider> Alright
< meshcollider> Yeah I don't see a particularly clean solution otherwise
< meshcollider> Ok let's end the meeting there then, too many people away today
< meshcollider> #endmeeting
< lightningbot> Meeting ended Fri Jun 21 19:46:24 2019 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< achow101> this refactor has turned out to be much harder than I thought it would be
< achow101> good luck reviewing it
< bitcoin-git> [bitcoin] asood123 opened pull request #16265: Update check disk space in bitcoind (master...patch-15813) https://github.com/bitcoin/bitcoin/pull/16265
< gwillen> so I just ran into an issue (twice) where I broke one of the python tests in a confusing way with a typo
< gwillen> and ended up accidentally reusing an intermediate from a testcase in a subsequent test case without noticing it
< gwillen> my friend the python expert strongly recommends that the test cases should be separate functions as the best way to avoid this kind of issue, which doesn't seem crazy to me
< gwillen> but of course, it seems like most or all the tests are written in this style
< gwillen> so I'm curious what people think about that
< gwillen> the failure mode where one accidentally reuses an intermediate from a previous test case is EXTREMELY mystifying and hard to diagnose
< gwillen> and it's easy to induce when adding/removing or commenting or uncommenting lines, given that each testcase uses most of the same names for intermediates
< sipa> gwillen: my favorite *ahum* bug i ever had in python was when i had two classes with the same name, and the second just overwrote the first one, without warning
< gwillen> oh geez
< sipa> making me mystified why some obvioisly broken code in the first one wasn't causing problems
< gwillen> yeah.
< sipa> even google's internal linter at the time did not catch this
< gwillen> when I suspect something like that my next move is always to break something VERY blatant in the code I'm not sure is running, like make it throw on the first line
< sipa> yeah
< sipa> anyway, your suggestions sounds very reasonable
< sipa> but i'm not a python person myself
< gwillen> my friend is one of the maintainers of mypy, and one of the Python People at dropbox
< gwillen> so I take his suggestions on python style seriously :-)
< sipa> ping MarcoFalke ^
< gwillen> (he is also the author of "Peer-to-Peer Affine Commitment using Bitcoin", which describes the "Typecoin" system, and was almost named "Massively Multiplayer Online Linear Logic")
< sipa> lol
< sipa> mmoll
< luke-jr> sdaftuar: ping
< jtimon> In regtest, I'm sending coins from bitcoin core to lightningd's newaddr but it doesn't seem to be receiving anything, I'm generating more blocks on the core node and calling dev_rescan_outputs on the lightning node, but still not seeing anything in listfunds and fundchannel says "Cannot afford transaction"
< jtimon> what may I be doing wrong?
< jtimon> sorry, wrong channel