< dmkathayat> Just getting started on core development and while reading through the functional tests, found a test rpc_net that isn't following the code style guidelines.
< sipa> dmkathayat: that is definitely possible
< dmkathayat> run_test() must come towards the end of the subclass? This one has it somewhere in the middle before helper methods. I can submit a Trivial PR.
< sipa> i'm glad to see your enthousiasm, but please don't
< sipa> * Do not submit patches solely to modify the style of existing code.
< dmkathayat> Got it!
< dmkathayat> I might pool it in with the long-term PR I'm starting on.
< fanquake> dmkathayat What are you working on in the long term PR?
< dmkathayat> sdaftuar has raised this issue in the past: https://github.com/bitcoin/bitcoin/issues/14210
< dmkathayat> I'm just starting to look at it. A testnet that could spin up a bunch of nodes with random IPs to test communication with outbound peers?
< fanquake> dmkathayat cool
< dmkathayat> Right now, I have a rudimentary setup using Docker containers. But here's an initial approach:
< dmkathayat> 1) Create example_testnet.py in functional tests
< dmkathayat> 2) Configure a Dockerfile and docker-compose.yml to have a bunch of nodes(initially a fixed number) and put them inside data/
< dmkathayat> 3) Run example_testnet.py that would start a docker instance as a subprocess, with each node running a sample test.
< dmkathayat> If this is vague to understand, I'm finishing a PR that'd explain it in more detail. Please bear with me.
< dongcarl> dmkathayat: I think it'd be better if the tests were runnable even on machines that don't have docker installed
< dongcarl> One route I was looking at today was doing this thru network namespaces
< dongcarl> Then on top of that, we basically want to do what Jepsen does, but perhaps without the Clojure dependency: https://github.com/jepsen-io/jepsen
< dmkathayat> dongcarl: thanks for your comment
< dmkathayat> Could there be a hard limit to how complex a topology can be if we use network namespaces?
< dmkathayat> I imagine a docker-like solution could scale to any number of nodes and allow for all sorts of complex behavior
< dongcarl> dmkathayat: Hahaha the point of the network namespace _is_ so that we can make the topology as complex as we want
< dmkathayat> dongcarl: Hmm. So you're suggesting one could scale to a complex enough topology on their machine if they wanted?
< dongcarl> But maybe docker can work just as well, we need to think about how we'd interrupt comms between containers or simulate packet loss and such
< dongcarl> By the way...
< dongcarl> docker is using network namespaces behind the scenes
< dongcarl> with a veth to your default route iface I think
< dmkathayat> I was coming from the fact that docker would allow anyone to do that (take it to a cloud if they want and do advanced stuff). If namespaces can still do it, sure.
< dmkathayat> I don't know much about network namespaces, but I guess there should be a hard limit if you're just limited to one machine? Got to read up more on it
< dongcarl> dmkathayat: Well if you look deep enough, docker doesn't scale to "any number of nodes," it uses network namespaces as a backend
< dongcarl> Also nodes have to refer to each other by their IP addresses, so that's inherently limited
< dongcarl> I think by default every new namespace has loopback
< dongcarl> so that's a /8 for you
< dmkathayat> It may be the other way round. Docker allows you to refer other nodes by their hostname. That's how I've used it in the past.
< dmkathayat> IP addrs work as well
< dongcarl> hostnames are resolved to IPs
< sipa> dmkathayat: they may have a hostname each, but they still need a unique ip address too
< sipa> i also don't think you'll be running 256 bitcoinds on any reasonably-sized machine any time soon...
< sipa> so i doubt this is a problem
< dongcarl> Haha true
< dmkathayat> sure, yeah. I was wondering how big this needs to be. You said it! :)
< sipa> (or would there just be a single actual bitcoind, and a bunch of python-backed simulations for the other IP addresses?)
< gmaxwell> I'm not entirely sure how useful that sort of thing is, since those hosts will be in private space (e.g. RFC1918) and bitcoind handles those addresses specially.
< dongcarl> The other thing to consider is the issue brought up by sdaftuar, that bitcoind behaves differently when connecting to nodes on private/local subnets thru `IsLocal` detection
< gmaxwell> jinx
< dongcarl> Yup
< dongcarl> But perhaps in your own namespace you can just assign IPs to whatever you want?
< sipa> i'm sure things can be coerced to look like routable IPs
< gmaxwell> they can, at some risk of breakign crap for people running the tests.
< dongcarl> Not if they're in their own namespace!
< dongcarl> If they're in the root namespace... of course
< gmaxwell> well okay true, but then causing kind of inexplicable behavior when you can't reach the nodes you're testing because they're namespaced. :P
< * gmaxwell> proposes to assign them to the subnets of popular ad networks so at least if they leak they'll improve your browsing expirence by being unreachable on port 80/443.
< sipa> hahaha
< dongcarl> lolll
< dongcarl> we'll just document it well, and it'll be fine I think
< dmkathayat> I guess having some kind of isolation definitely helps
< dongcarl> I really wish we could integrate something like Jepsen, it seems quite flexible, but the extra dependency for people who want to test might be a no-go
< gmaxwell> please don't make some kind of vm/system image/anything that requires root be a pre-req for tests in general. For a special test that not everyone has to run, okay dokie, I'll just never run it.
< dongcarl> Yup, agreed
< dmkathayat> dongcarl: wouldn't you rather prefer having a docker dependency? And if docker uses namespaces anyway, why should we reinvent the wheel?
< dongcarl> dmkathayat: network namespaces ship with most Linux kernels
< dmkathayat> gmaxwell: thanks for chiming in with `isLocal` reference.
< dongcarl> docker does not and requires root
< dongcarl> (actually not sure if network namespaces require root but at least no dependency)
< sipa> pretty sure they do
< dongcarl> Ah, they require you to be added to `netns` group
< sipa> ah
< dmkathayat> dongcarl: are you working on network namespaces already? I'd love to help anyway I can. Tbh, I just wanted to get started on bitcoin
< dmkathayat> (if the `consensus` is not to go after docker.. haha)
< dongcarl> dmkathayat: Haven't started on it yet, feel free to do it
< * sipa> sees the word docker and gets an irrational "why does everything need to be in vms/containers/...?" reaction
< dmkathayat> dongcarl: sure
< sipa> (but given that i hardly know anything about it, you should probably disregard my opinion)
< jarthur> With network namespaces, do you get your own abstract namespace as well in Linux? You'd get a few million network addresses in that namespace once linux abstract sockets are supported.
< dongcarl> Not sure I understood the question
< sipa> jarthur: you're referring to unix sockets? they're not really a solution here, as the goal is testing a.o. the addrman peer selection logic etc
< sipa> which is specific to the public internet
< jarthur> got it, am still curious re the abstract namespace :)
< dongcarl> I don't know much about abstract namespaces sry
< dongcarl> Perhaps you can enlighten?
< dongcarl> Ah wait
< jarthur> They're like file path Unix sockets, except are known only to the networking stack and not the file system, and are permisionless. Very high performance to work with and you get a massive number of possible address combinations, but in the end, yea, Unix socket family and not applicable to this use case.
< jarthur> Not supported by Windows unix sockets or macOS either
< dongcarl> "network namespaces isolate the UNIX domain abstract socket namespace"
< jarthur> sweet
< dmkathayat> sipa: the issue sdaftaur had created (14210) had "containers" somewhere in there. I translated that to mean docker :P
< sipa> dmkathayat: ha, that's fait
< jarthur> I'm taking a look at unix socket support for RPC again, am trying to figure out the best way to convey an abstract socket in bitcoind.conf or at the command line. In Linux, you prepend the socket path name with a null byte to hint to the kernel that it's abstract, but it seems wrong to have folks enter a control char at the command line or in the conf file even if it is valid UTF-8.
< jarthur> Maybe better to not support it at first, figure it out later.
< dongcarl> jarthur: Maybe take a look at how ssh does it?
< dongcarl> I know some apps use `unix://` vs `tcp://` to differentiate
< dongcarl> but you probably just add a separate option if you want
< jarthur> Looks like procfs and curl use @ symbol. Couldn't find an escape sequence for OpenSSH; it may not support it officially.
< bitcoin-git> [bitcoin] ken2812221 closed pull request #14464: refactor: make checkqueue manage the threads by itself (also removed some boost dependencies) (master...drop-boost-cond) https://github.com/bitcoin/bitcoin/pull/14464
< fanquake> Have i jumped the gun with #15584 ? Otherwise looking for a Concept ACK
< gribble> https://github.com/bitcoin/bitcoin/issues/15584 | build: disable BIP70 support by default by fanquake · Pull Request #15584 · bitcoin/bitcoin · GitHub
< fanquake> wumpus Are we going straight to rc3 given that #15602 has been merged?
< gribble> https://github.com/bitcoin/bitcoin/issues/15602 | 0.18: [p2p] Enable reject messages by default by jnewbery · Pull Request #15602 · bitcoin/bitcoin · GitHub
< fanquake> eh, guess we dont actually have to skip ahead.
< wumpus> huh no I didn't tag rc2 yet did I?
< wumpus> everyone seems to be so convinced that I did that I'm starting to doubt myself lol
< gmaxwell> lol
< gmaxwell> sorry, my fault I saw you bump the version and thought that was the tagging.
< fanquake> wumpus yea I thought it'd been done after seeing the version bump in the branch, sorry
< gmaxwell> hurrah, I'm not the only gun jumper
< wumpus> jnewbery MarcoFalke please update the release notes in the wiki, not in the tree!
< wumpus> if people edit them in two places this leads to extra work to unify them again, and changes might be accidentally lost
< sipa> wumpus: if you're talking about #15604, that's for the release notes in master (for 0.19)
< gribble> https://github.com/bitcoin/bitcoin/issues/15604 | [docs] release note for disabling reject messages by default by jnewbery · Pull Request #15604 · bitcoin/bitcoin · GitHub
< gmaxwell> sipa: no the PR that MarcoFalke merged earlier
< gmaxwell> sorry, I should have squaked about that
< fanquake> carldong I've got the guix build happening (env setup atm) in an alpine docker container. Seems to be running alright so far.
< wumpus> sipa: commit a7563633d2 on 0.18
< wumpus> sipa: for master you're right, of course
< wumpus> anyhow, time to tag rc2
< fanquake> \o/
< bitcoin-git> [bitcoin] laanwj pushed tag v0.18.0rc2: https://github.com/bitcoin/bitcoin/compare/v0.18.0rc2
< provoostenator> Gitian started!
< fanquake> That's efficient
< sipa> the taggening is upon us
< nigs> Hey guys, I don't know if this is the right channel for my question. Anyway, I'm trying to develop a cryptocurrency from ground up, but I don't know where or how to start. Could anyone point me in the right direction?
< bitcoin-git> [bitcoin] jonasschnelli pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/b83c6f79400f...d67f9d0db98e
< bitcoin-git> bitcoin/master 228e806 JeremyRand: Enable TLS in link to chris.beams.io
< bitcoin-git> bitcoin/master d67f9d0 Jonas Schnelli: Merge #15577: Docs: Enable TLS in link to chris.beams.io
< bitcoin-git> [bitcoin] jonasschnelli merged pull request #15577: Docs: Enable TLS in link to chris.beams.io (master...chris-beams-tls) https://github.com/bitcoin/bitcoin/pull/15577
< provoostenator> nigs: definitely not the right channel
< provoostenator> Try Twitter, though brace yourself :-)
< Sentineo> shoot
< Sentineo> just before finishing gitian build I ran out of space :D
< Sentineo> using docker, is there a way to continue, or I will have to start from scratch. I will add more GB to the vm.
< dongcarl> Sentineo: Are you using gitian inside a VM?
< dongcarl> so like docker-gitian inside VM?
< provoostenator> I don't think Docker guaranteers containers to be isolated.
< dongcarl> provoostenator: ? I think he's running gitian inside a VM as per the instructions, and the VM has low disk space
< Sentineo> dongcarl: yes
< provoostenator> Normally there should not be a need to run Docker in a VM.
< dongcarl> DougieBot5000: Right, resize the disk in your vm manager, then use something like resize2fs
< dongcarl> Sentineo:
< Sentineo> it is pretty slow, so I might just try running it on the vm it self. Tough docker would give me less headache
< dongcarl> Sentineo: gitian always requires a vm/container dependency
< provoostenator> The instructions might be out of date. Afaik you don't need a VM if you're using the new Docker approach.
< dongcarl> Sentineo: Right, just lose the outside VM
< wumpus> I like using the outside VM so that the bare metal setup is not cluttered with all kinds of dependencies and LXC configuration etc, but it's definitely not necessary
< dongcarl> wumpus: agreed
< provoostenator> wumpus: that's for LXC, not for Docker, right?
< dongcarl> provoostenator: Guess it depends on what dep you prefer on your bare metal, a VM manager or docker
< Sentineo> hm lxc vs docker, wondering how much performance difference is it
< provoostenator> Afaik Docker has very little overhead, as long as you give it access to your real disk rather than a virtual volume.
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/d67f9d0db98e...118a5c8d94de
< bitcoin-git> bitcoin/master 19a0c4a Carl Dong: depends: native_protobuf: avoid system zlib
< bitcoin-git> bitcoin/master 118a5c8 MarcoFalke: Merge #15580: depends: native_protobuf: avoid system zlib
< Sentineo> just so I have an idea. How long does it take for you to do a gitian build? linux+windows for example
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15580: depends: native_protobuf: avoid system zlib (master...2019-03-depends-native_protobuf-no-zlib) https://github.com/bitcoin/bitcoin/pull/15580
< wumpus> provoostenator: yes, I don't know about docker
< wumpus> gitian builds are extremely slow here, but I don't blame the VM overhead for that, the ubuntu "upgrading packages" step takes very long which the inner OS is messing around with package management
< wumpus> also it simply does a lot of work, a full build of bitcoind + qt for many different platforms ! even more if it the dependencies aren't cached and it needs to build qt
< wumpus> (e.g.the qt library not just bitcoin-qt)
< harding> As part of testing RC1, I was using the new bitcoin-wallet with its "info" command to inspect some backups, but I noticed that it changes the file write times (and won't work if I run on a read-only wallet file). Does it actually need write access to print basic wallet info? I'd really prefer inspections be read only (and work with read-only files).
< wumpus> harding: in theory: no, it doesn't need write access to inspect files; in practice it might be the case that BerkeleyDB always opens files read-write, but I don't know this!
< wumpus> would be interesting to try to run it on a wallet on a read-only fs
< bitcoin-git> [bitcoin] jamesob opened pull request #15606: [experimental] UTXO snapshots (master...utxo-dumpload-compressed) https://github.com/bitcoin/bitcoin/pull/15606
< wumpus> in any case this is something it makes sense to open a github issue for
< harding> wumpus: would you expect a read-only fs to differ from my test of just chmod 400'ing the file?
< harding> I'll open a feature request issue. Thanks, wumpus!
< wumpus> harding: I think those are more or less equivalent, from a user-space point of view
< wumpus> (then again I don't know what berkeleydb does internally)
< harding> Yeah, I'd think the same. In my test, if I try running on a read-only file (ext4 fs), bitcoin-wallet says it can't open and asks if the file is in use by another process.
< wumpus> anyhow I think it's a reasonable request, and is probably straightforward to fix *if* berkeleydb gives control over these kind of open flags
< bitcoin-git> [bitcoin] jonatack opened pull request #15607: Release process updates (master...release-process-updates) https://github.com/bitcoin/bitcoin/pull/15607
< wumpus> FWIW there's a DB_RDONLY flag for opening databases
< harding> Issue opened, #15608
< gribble> https://github.com/bitcoin/bitcoin/issues/15608 | Feature request: bitcoin-wallet tool: dont modify files unless requested · Issue #15608 · bitcoin/bitcoin · GitHub
< wumpus> thanks!
< harding> Thank you!
< fanquake> Sentineo if you've gitian building on macOS, I've got some native Docker instructions here: https://github.com/fanquake/core-review/tree/master/gitian-building
< cfields> dongcarl / ryanofsky: there's a lot of history in 14856 about GetDefaultPort() that I can't quite follow. Maybe you could briefly explain why it's still needed?
< cfields> Sorry, GetListenPort()
< dongcarl> 1st try: Removed GetDefaultPort and required mapPort to take in a port as its second parameter: `virtual void interfaces::Node::mapPort(bool, uint16_t)`
< cfields> The obvious problem there is that it's now a global and shared between two instances.
< dongcarl> This didn't work because qt called `mapPort` and didn't have the listen_port
< dongcarl> 2nd try: Use `g_connman->GetDefaultListenPort()` to get the port inside `mapPort` and remove its second parameter that we added in first try
< dongcarl> This didn't work because if `interfaces::Node::mapPort` is called before `CConnman::Start`, there will be a garbage default value
< dongcarl> cfields: Does that make sense?
< cfields> Yup
< cfields> but
< cfields> why not just pass the port into mapPort from the callsites? Which would mean passing it to qt during init as well.
< cfields> StartMapPort(uint16_t default_port)
< cfields> void mapPort(bool use_upnp, uint16_t default_port)
< dongcarl> So you mean, continue with 1st try and just make sure that qt has `listen_port` to pass in to `mapPort`?
< cfields> Sure. As-is, the change seems like a lateral move at best :(
< dongcarl> cfields: That's true
< cfields> Because the intention is to get rid of the globals so that instances respect their own port values. With this change, they'd still collide.
< cfields> That said...
< cfields> nm.
< cfields> dongcarl: I haven't looked at the code in depth in a while, but I don't think there's anything that would keep that from working. Mind giving it a shot?
< cfields> Otherwise, imo the scope of the PR should just be reduced to maybe torport and default outgoing.
< dongcarl> Yeah I'd be happy to give it a shot. Question tho: maybe I'm thinking about it the wrong way, but logically, why should qt be parameterized by a port number?
< cfields> It shouldn't necessarily. But it makes sense to init all subsystems with values that they'll need to start up. Even if that means duplicating it to pass to a few different subsystems.
< cfields> (Imo anyway. But that's very much a style preference)
< dongcarl> Okay, I'd be happy to give it a go
< dongcarl> Not familiar with qt at all so if someone can point me to where to pass things in, that'd be helpful
< cfields> Thanks!
< cfields> Heh, I'm not either. I think I might've either skipped that or cheated with my original PR.
< cfields> jonasschnelli: ^^ :)
< cfields> back in a bit
< provoostenator> fanquake: thanks for sharing the link to Docker instructions for macOS
< provoostenator> Wallet meeting?
< sipa> oh, i thought next week
< luke-jr> even if today, it's an hour early
< sipa> ah yes
< provoostenator> Ah yes, off by one hour. But I believe the last one was two weeks ago. We shifted them a bunch of times.
< meshcollider> Yep it should be today I believe
< meshcollider> sipa: can you please host it today? I'll be around but my internet is patchy again
< sipa> i can't
< meshcollider> Someone else then, provoostenator ?
< provoostenator> I'm here, but I don't I have IRC powers, and I don't know the incantations. achow101?
< meshcollider> I dont think you need any permissions, I dont have any either :)
< provoostenator> Ok, I'll see what happens then...
< meshcollider> I think the only commands you need are startmeeting, topic, action, and endmeeting
< meshcollider> Thanks
< jonasschnelli> cfields: osx signatures are up
< provoostenator> #startmeeting
< lightningbot> Meeting started Fri Mar 15 19:00:04 2019 UTC. The chair is provoostenator. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< provoostenator> #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 gleb
< provoostenator> Topics for this week?
< meshcollider> any wallet high priority PR changes wanted?
< instagibbs> already added to regular list
< achow101> hi
< provoostenator> Sounds like we can keep this meeting short?
< meshcollider> doesnt seem like theres much to talk about, indeed :)
< achow101> what's the status on the descriptor wallet stuff?
< provoostenator> I'm in the middle of rewriting stuff for descriptor wallets, so not much to show now.
< achow101> and/or wallet overhaul
< provoostenator> #topic descriptor wallets
< achow101> the current desciptor wallet pr is just for having a descriptors record, right?
< achow101> it's not the full overhaul "use descriptors everywhere"
< instagibbs> nor ismine overhaul?
< provoostenator> I'm not sure yet.
< provoostenator> I definately want a proof of concept that is usable, but it might be split over multiple PRs.
< meshcollider> I think sipa is planning on doing the ismine overhaul
< sipa> yeah
< sipa> but i'm in the middle of a few other things right now, so if someone else wants to have a shot
< provoostenator> So I'll probably keep this one focussed on serialization, wallet flags and import commands.
< achow101> what needs to be done for ismine?
< sipa> achow101: my idea is to abstract out the wallet's keypool+ismine logic, and then descriptor records become an alternative implementation of that interface
< provoostenator> AddToWalletIfInvolvingMe
< achow101> so ismine becomes "this script exists in my keypool/wallet/whatever"?
< meshcollider> handling ismine in a similar way to how signingprovider abstracts signing
< provoostenator> Should probably loop over descriptors and call something there.
< sipa> provoostenator: descriptor records would have a cache of sPKs to look for
< sipa> as they can be pre-expanded
< sipa> which replaces the keypool concept
< sipa> provoostenator: no, all of IsMIne, not just AddToWalletIfInvolving me (i don't expect that function to change much)
< meshcollider> the cache is already implemented in one of your earlier PRs right
< sipa> sure, but descriptor records aren't
< achow101> so (at a high level) all that's left is storing descriptors in the wallet and ismine logic
< sipa> "all that's left" haha
< achow101> (which I guess is "everything")
< sipa> but yes :)
< provoostenator> We also need to think about how to store labels and how to store which addresses have been "reserved", e.g. though getnewaddress.
< sipa> labels wouldn't change at all
< achow101> provoostenator: I don't think that would be much (or any) different from how we do it now
< sipa> and reserved addresses are in the records, which store how far each has been explored so far
< provoostenator> Ah I see address labels are literaly indexed by the address string.
< achow101> in other news, hwi 1.0 will be released today just in time for people to use it with core 0.18
< provoostenator> #topic hardware wallets
< provoostenator> achow101: nice!
< provoostenator> Any other topics?
< achow101> seems not
< provoostenator> #endmeeting
< lightningbot> Meeting ended Fri Mar 15 19:22:18 2019 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< gmaxwell> \O/
< provoostenator> gmaxwell: too soon?
< cfields> gitian builders: detached sigs for v0.18.0rc2 are up.
< gmaxwell> provoostenator: no not too soon, that was just random cheering.
< gmaxwell> (I was around for the meeting but didn't have anything to say)
< gwillen> are we permanently offsetting wallet meeting parity
< sipa> gwillen: no, it's you who has moved
< sipa> (meeting is 7pm GMT)
< achow101> sipa: I think we're offset by a week
< sipa> ah!
< sipa> sorry, i thought it was about the time
< gwillen> "no, it's the children who are wrong"
< gwillen> but yeah I meant the week offset
< gwillen> I think it can be tough to keep track of such things
< gwillen> I wonder how many people besides me were expecting it to be next week
< promag> can someone tell me why InterpretBool("true") gives false?
< gwillen> ... :-(
< gwillen> because it only interprets numbers
< gwillen> "return (atoi(strValue) != 0);"
< gwillen> atoi returns zero for strings with no numeric prefix
< gwillen> actually there is a long sad comment about this above the definition of InterpretBool in system.cpp
< promag> o wait for it k
< fanquake> provoostenator no worries