Correctly stringify meta-addresses, and bind to all addresses

[christianbundy]

Previously each multiserver interface was only allowed to output one address, which led to unexpected behavior when { host: '::' } was configured.

This commit changes the stringify() method when meta-addresses are passed, which now outputs a semicolon-delimited string with all of the addresses that Node.js is listening on. This should make meta-addresses usable and makes it unnecessary to enumerate all possible addresses when configuring multiserver.

Before

combined.stringify('local') // =>
// net:192.168.3.55:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=

After

combined.stringify('local').split(';').join('\n') // =>
// net:192.168.3.55:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=
// net:172.18.0.1:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=
// net:fce2:9811:4862:81a7:bb08:91d6:2e41:d220:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=

---

Status: ready for review, should make https://github.com/ssbc/ssb-config/pull/53 obsolete.

---

This also resolves an issue where the module wasn't able to be parsed in strict mode.

See: ssbc/ssb-server#683

[christianbundy]

Seems to be working great now, but how should we add tests for this? We can't hardcode addresses because it will work differently on all machines. Maybe compare against getAddresses(null, scope) output?

# meta-address returns multiple
[ 'net:192.168.0.109:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=',
  'net:172.18.0.1:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=',
  'net:fce2:9811:4862:81a7:bb08:91d6:2e41:d220:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=' ]
[ 'ws://192.168.0.109:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=',
  'ws://172.18.0.1:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=',
  'ws://[fce2:9811:4862:81a7:bb08:91d6:2e41:d220]:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=' ]

[christianbundy]

I got carried away and fixed the Unix socket implementation on Windows as well. Oops?

Travis is green again! :tada:

[christianbundy]

Pulled in the latest changes from master and followed the above suggestions from @staltz and @mixmix. Would love another round of review on this, when y'all have time. :eyes:

[staltz]

@christianbundy Thanks for rebasing. I took another look at this and the PR feels big, in the sense that it's making many changes at once. Could you split it in multiple PRs? For instance "Correctly stringify meta-addresses" and then "Fix Unix sockets on Windows", but I'm sure there are other PRs to be made too, e.g. style changes or mere file renaming/moving.

The change from one net address to multiple net addresses is influential too. I'm not sure how the rest of the SSB stack will react to this. maybe there's somewhere where they assume there's only one net address, just maybe. I think this used to be the case with ssb-gossip, but not anymore with ssb-conn, because ssb-conn only assumes there are multiserver addresses (that conform to multiserver-address). But who knows, maybe somewhere there's an assumption of one address. Or maybe someone is using ssb-gossip and could update multiserver to use this PR. I also feel like the new file lib/network.js looks too similar in purpose to non-private-ip. It's important to keep this module (and all others) modular and not monolithic.

[christianbundy]

Thanks for the quick review @staltz.

I've opened #57 with some cherry-picked commits specific to the Windows fix, I'd love your feedback in that PR. Merged!

The change from one net address to multiple net addresses is influential too.

This is the behavior that I'd expect from the module, but we can release this as a major version bump if you think there's a high risk for it to break things. I'd rather err on the side of safety and compatibility rather than try to shove a big change into a patch commit. :beetle:

I also feel like the new file lib/network.js looks too similar in purpose to non-private-ip.

Regarding filterNetworkInterfaces(), I think there's an important difference: non-private-ip promises to "returns the first address" that it sees, whereas filterNetworkInterfaces() returns an array of addresses (with filtering options for internal and family properties). If there was a bug in non-private-ip I would've created a pull request, but I don't think the API of the module matches our needs.

The protocolToAddress() and getRandomPort() functions in lib/network.js had been written a few different times around this module, so I thought it was best to consolidate them in one place.

The other relevant function in lib/network.js is getAddresses(), which takes a host and scope as input and returns an array of hosts that we'll be listening on. This replaces multiserver-scope, which is essentially a switch...case that calls non-private-ip.

I made a small graph to visualize the change to the dependency graph:

[dominictarr]

@staltz everything in sbot understands proper multi addresses now, since the last time I worked on ssb-gossip... in april

[christianbundy]

Would it be helpful to have a call or something similar to walk through this PR? Or is there anything I can do to help move it forward? I think GitHub is showing compose.js as a much bigger diff than it really is, since the file was also renamed. I'll add the real diff below, if that's helpful.

click me

```diff diff --git a/compose.js b/lib/compose.js index 2c7a103..8178bd8 100644 --- a/compose.js +++ b/lib/compose.js @@ -1,3 +1,5 @@ +const cartesianProduct = require('cartesian-product') + var separator = '~', escape = '!' var SE = require('separator-escape')(separator, escape) @@ -46,20 +48,56 @@ function asyncify(f) { } } -module.exports = function (ary, wrap) { +// We can't call `cartesianProduct()` directly because it expects `[layer[]?]`. +// +// Sometimes this function is called with the correct inputs, but often it's +// +// - `undefined` +// - `[undefined, layer]` +// - etc +// +// Since the `cartesianProduct()` function fails on any expected inputs, this +// function ensures that we're always passing a `[]` or a 2-dimensional array. +const combineLayers = (input = []) => + cartesianProduct( + input.map((item) => + Array.isArray(item) ? item : [] + ) + ) + +module.exports = function (layers, wrap) { if(!wrap) wrap = function (e) { return e } - var proto = head(ary) - var trans = tail(ary) + var protocol = head(layers) + var transform = tail(layers) function parse (str) { - var parts = SE.parse(str) - var out = [] - for(var i = 0; i < parts.length; i++) { - var v = ary[i].parse(parts[i]) - if(!v) return null - out[i] = v - } - return out + // We need to parse addresses that may be delimited by semicolons. The goal + // is to return the first address that's understood by all layers. For + // example, if we're using the `net` and `shs` plugins then we need to + // ensure that **both** plugins can understand the multiserver address that + // we listen on. + return str.split(';') + .map((singleAddress) => { + // First we split the multiserver address into parts. + // This might look like: [ "net:localhost:8008", "shs:abc" ] + const parts = SE.parse(singleAddress) + + // Next we need to ensure that all of our layers can parse their part. + const parsedParts = parts.map((part, index) => layers[index].parse(part)) + + // The layer parsers don't throw errors, so we need ot make sure that + // they return a truthy value for all parts. + const allLayersHandled = parsedParts.every((part) => part) + + if (allLayersHandled) { + return parsedParts + } else { + return null + } + }).find(option => + // Find the first option where all layers are handled. + option !== null + ) || null // Fallback to `null` if we can't correctly parse this address. } function parseMaybe (str) { @@ -67,16 +105,16 @@ module.exports = function (ary, wrap) { } return { - name: ary.map(function (e) { return e.name }).join(separator), - scope: proto.scope, + name: layers.map(function (e) { return e.name }).join(separator), + scope: protocol.scope, client: function (_opts, cb) { var opts = parseMaybe(_opts) if(!opts) return cb(new Error('could not parse address:'+_opts)) - return proto.client(head(opts), function (err, stream) { + return protocol.client(head(opts), function (err, stream) { if(err) return cb(err) compose( wrap(stream), - trans.map(function (tr, i) { return tr.create(opts[i+1]) }), + transform.map(function (tr, i) { return tr.create(opts[i+1]) }), cb ) }) @@ -89,10 +127,10 @@ module.exports = function (ary, wrap) { console.error('server error, from', err.address) console.error(err.stack) } - return asyncify(proto.server(function (stream) { + return asyncify(protocol.server(function (stream) { compose( wrap(stream), - trans.map(function (tr) { return tr.create() }), + transform.map(function (tr) { return tr.create() }), function (err, stream) { if(err) onError(err) else onConnection(stream) @@ -103,13 +141,19 @@ module.exports = function (ary, wrap) { parse: parse, stringify: function (scope) { var none - var _ary = ary.map(function (e) { - var v = e.stringify(scope) + + var identifierAry = combineLayers(layers.map(function (item) { + var v = item.stringify(scope) + if (typeof v === 'string') { + v = v.split(';') + } + if(!v) none = true else return v - }) + })) + if(none) return - return SE.stringify(_ary) + return identifierAry.map(id => SE.stringify(id)).join(';') } } } ```

[staltz]

@christianbundy I don't have time budget these weeks for properly reviewing this. We need to ask others.

Alternatively, one thing I do quite often with Manyverse is I fork the package and use the fork in my development, and then production. This works as an integration test before merging into the official repo. For instance I'm doing that with ssb-suggest-fork for the PR https://github.com/ssbc/ssb-suggest/pull/6 and then I can tell in the PR that "I am absolutely sure this works without significant downsides" and there is less guesswork to do when reviewing the PR. The reviewer doesn't have to mentally predict for bugs if the code is already tested in the wild.

I feel similarly about this PR for multiple addresses. I don't know how (old versions of) Patchwork and (old versions of) Manyverse are going to react to it when broadcasting LAN addresses. I'd like to see this PR working in the real world. So my best suggestion to get this PR merged is to use it in the wild. Maybe you can publish @christianbundy/multiserver or multiserver-fork on npm and utilize it in Oasis, and see how it goes, does it communicate well with Patchwork LAN, with Patchbay LAN, with Manyverse LAN, with room peers, etc. Then you can report back with test results of interoperability with those clients. Etc

[christianbundy]

Thanks for the update, I appreciate it.

Some useful context: if you're using SSB-Config you're using an emulation of this PR, which was merged in October, and we even found a bug in it (!), which has a pending PR too.

I don't think the pain point is specific to this PR, and I wrote about it a bit on SSB, but I agree that the "temporary fork" approach is an option. Unfortunately depending on a Git branch seems like it may break npm install, so I think the 'publish on npm' idea would be an improvement there. I'd really love to avoid having to temporarily fork all of the modules I'm working on, but I think this is worth fixing from a zoomed-out 'development protocol' view instead of just this PR.

I'll do a temporary fork of Multiserver + SSB-Config + SSB-Client and publish on npm, thanks a bunch for that tip.

[christianbundy]

On second thought, I'm not sure that the 'temporary fork' approach will work:

$ npm ls multiserver
@fraction/oasis@2.10.0 /home/christianbundy/src/fraction/oasis
├─┬ @fraction/flotilla@3.0.0
│ ├─┬ secret-stack@6.3.1
│ │ └── multiserver@3.6.0  deduped
│ ├─┬ ssb-conn@0.11.3
│ │ └─┬ ssb-conn-hub@0.2.7
│ │   └── multiserver@3.6.0  deduped
│ ├─┬ ssb-no-auth@1.0.0
│ │ └── multiserver@3.6.0  deduped
│ ├─┬ ssb-onion@1.0.0
│ │ └── multiserver@3.6.0  deduped
│ ├─┬ ssb-room@1.1.1
│ │ └─┬ secret-stack@6.3.0
│ │   └── multiserver@3.6.0  deduped
│ ├─┬ ssb-unix-socket@1.0.0
│ │ └── multiserver@3.6.0  deduped
│ └─┬ ssb-ws@6.2.3
│   └── multiserver@3.6.0  deduped
└─┬ ssb-client@4.7.9
  └── multiserver@3.6.0 

I could npm link for local development, but I don't know of a simple way that I could switch everything to depend on @foo/multiserver instead. I'll try patch-package instead, but as a development workflow this feels janky and brittle.

[christianbundy]

Just banged my head against patch-package for 90 minutes and I think I need to call it a day.

It looks like patch-package wants a diff between the contents of the npm tarball and the final directory, not the diff between the release source and the source from this branch. These are mostly the same, because we don't use a fancy compiler, but the package.json isn't quite the same and it requires manual intervention. I think I need to make SSB-Config patch Multiserver for my PR, plus I need to have Oasis patch both SSB-Config and Multiserver, and the end result is still just me forking the module.

When we can, I think it's important that we can zoom out a bit and (maybe not in this issue) discuss how we can improve the development of maintenance of our shared modules. :heart:

[christianbundy]

Just confirming that this works in Oasis, I added a dummy interface and can confirm that listening on :: works right and broadcasts the available addresses correctly:

net:192.168.1.217:8008~shs:+oaWWDs8g73EZFUMfW37R/ULtFEjwKN/DczvdYihjbU=
net:192.0.2.1:8008~shs:+oaWWDs8g73EZFUMfW37R/ULtFEjwKN/DczvdYihjbU=
ws://192.168.1.217:8989~shs:+oaWWDs8g73EZFUMfW37R/ULtFEjwKN/DczvdYihjbU=
ws://192.0.2.1:8989~shs:+oaWWDs8g73EZFUMfW37R/ULtFEjwKN/DczvdYihjbU=

(I've replaced ; with newlines for readability).

I can also confirm that my Raspberry Pi running SSB-Server and my Android phone running Manyverse can see my desktop device. Are there any other tests that anyone would like me to perform?

[dominictarr]

this PR is very big, which makes it hard to tell whats going on. Changing things like strict mode parsing makes the diff noisier. did compose change? since you moved it into lib github just says it was deleted and a new file was added. That also means later when we want to debug something we can't use git blame to figure out when a specific line was added.

Should passing 0.0.0.0 to multiserver automatically create many bound addresses? or should multiserver only do exactly what you tell it? I understand why you need that, but this is a low level library. I think it would be better to put the "helpful" stuff in the ssb layer, and maybe even error or warn in multiserver if it gets a weird :: or 0.0.0.0 address.

[christianbundy]

this PR is very big, which makes it hard to tell whats going on. Changing things like strict mode parsing makes the diff noisier. did compose change?

Thanks for taking another look at this PR. Check out the real diff for compose that I posted in this comment. It's pretty small, and I think the only "noise" is just me renaming variables that I found ambiguous/confusing:

• ary => layers
• proto => protocol
• trans => transform

That also means later when we want to debug something we can't use git blame to figure out when a specific line was added.

My version of Git still shows the correct blame, as does GitHub's blame on that file. Is your local Git not showing a blame on that file?

Should passing 0.0.0.0 to multiserver automatically create many bound addresses?

That's the current behavior, and the basis of this pull request, but we could scrap this PR and make a breaking change instead.

or should multiserver only do exactly what you tell it? I think it would be better to put the "helpful" stuff in the ssb layer, and maybe even error or warn in multiserver if it gets a weird :: or 0.0.0.0 address.

My gut reaction is "NO! GOD! NO! GOD, PLEASE NO! NO! NO! NOOOOO!", but I might just be another victim of the sunk cost fallacy. Could you help me understand which problem(s) that would solve, or why that would be preferable to this change?

[christianbundy]

@dominictarr Also to answer your question here:

if you use :: as the address how does it behave when you do getAddress('public') getAddress('local') getAddress('device')? does it give the correct results?

I've only copied multiserver-scopes implementation, and haven't added any checks on top of that. I could add some logic to filter private address spaces (10.0.0.0, 172.16.0.0, 192.168.0.0, etc), but I was trying to keep this PR as minimal as possible.

Device

Literally just localhost.

[
  'net:localhost:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw='
]

Local

All non-private IP addresses.

[
  'net:192.168.1.217:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=',
  'net:192.0.2.1:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=',
  'net:123.45.67.89:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw='
]

Public

All non-private IP addresses, again.

[
  'net:192.168.1.217:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=',
  'net:192.0.2.1:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=',
  'net:123.45.67.89:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw='
]

[dominictarr]

@christianbundy that's wrong.

device is correct.

local should be private ip addresses. 192.168... is correct, that is a private address. the other addresses are non-private.

public should be non-private addresses only.

correct local

[
  'net:192.168.1.217:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw='
]

correct public

[
  'net:192.0.2.1:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=',
  'net:123.45.67.89:4848~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw='
]

[dominictarr]

btw, I checked those addresses using require('ip') I can't tell wether an address is private or not just by eyeball (except 192.168...)

[dominictarr]

oh, I just presumed that blame wouldn't work because the diff didn't look right. if blame works thats okay.

[dominictarr]

I suggested having the automatic stuff not in this module because I think it makes tools difficult to use if they try to help you too much, you end up working around their quirks. Tools should just do exactly what you tell them. Then, if you want helpful sugar, wrap that tool in another module. Then if you want the friendly interface, you can use it. but if it's getting in your way, you can use the low level thing directly. This also makes maintenance easier, because the low level concern is just correctness/performance/security etc. It clarifies what the objectives are. But the upper level concern is user-friendlyness. This way you don't have to trade off measurable/objective concerns with subjective.

It seems like you are solving a scuttlebutt problem, but this module isn't a ssb-* module. it should be generic. I consider this to be the case for everything that doesn't have a ssb-* prefix.

Sunk costs: would you really be loosing those costs if you just moved that code into another module?

[dominictarr]

Should passing 0.0.0.0 to multiserver automatically create many bound addresses?

That's the current behavior, and the basis of this pull request, but we could scrap this PR and make a breaking change instead.

but the current master branch is just a wrapper around net: https://github.com/ssbc/multiserver/blob/master/plugins/net.js#L42-L72 the current behavior you refer to comes from secret-stack or ssb-config, doesn't it?

[christianbundy]

@christianbundy that's wrong.

Thanks, I see now that when you check ip.isPrivate that's actually using a fun regex that checks whether it's available over LAN, not actually "private". I assumed that it was using the { internal } property from os.networkInterfaces(), but I can add that regex (or that library).

Tools should just do exactly what you tell them. Then, if you want helpful sugar, wrap that tool in another module.

I don't want helpful sugar, I want to listen on 0.0.0.0 or ::. This is a super common interface to listen on, and has been our default until we recently learned that Multiserver's stringify() handling broke when you listen on an "unspecified address".

but the current master branch is just a wrapper around net

Yes, and net.Server.listen() has native support for unspecified addresses. It sounds like you'd like to disable the native behavior from the OS and then emulate it in a helper library instead. This might be the right move, but I have no way of knowing unless you help me understand what problem you're solving.

the current behavior you refer to comes from secret-stack or ssb-config, doesn't it?

No, currently Multiserver is totally fine if you pass it an unspecified address like 0.0.0.0 or ::, it's just that stringify() doesn't work right for those addresses. Those modules have historically used :: as well, but support for unspecified addresses has been part of Multiserver for as long as I can remember.

[dominictarr]

@christianbundy oh, internal means device

https://devdocs.io/node/os#os_os_networkinterfaces

internal true if the network interface is a loopback or similar interface that is not remotely accessible; otherwise false

internal means localhost which means device. then ip.isPrivate means local.

What I was refering to is ssb-config recently changed to generate bindings for each interface if you didn't set anything. I also had to make a PR to fix scopes there. https://github.com/ssbc/ssb-config/pull/64/files oh so basically this PR is moving that feature into here. However, if you didn't set a host or configure incomings, it would work (and ms.stringify(scope) correctly) but if you set host to :: it would fail to give correct addresses out because it just passed :: to Server.listen.

somethings which depend on scopes working right:

• ssb-local should only broadcast a local address.
• ssb-device-address needs a public address, that is connectable from the internet.
• ssb-peer-invites ssb-invite needs a public address.

before this PR, multiserver doesn't actually know what scopes mean, they are just arbitary groupings. that is important some times, for example ssb-tunnel gives public scope addresses, connectable from the internet, but they don't have ip addresses in them. Also, tor hidden services are public, without ip addresses also.

We have always been able to bind to :: but that clearly breaks stringify. Support for that is a part of the networking stack, but unfortunately there isn't a thing "give me an address connectable in this context" (which is something multiserver is trying to solve)

So i think there is two options:

• throwing on :: and 0.0.0.0
• multiserver knows what scopes mean and sometimes you pass in one thing and get N things back.

I'm leaning towards the first.

[christianbundy]

What I was refering to is ssb-config recently changed to generate bindings for each interface if you didn't set anything. [...] oh so basically this PR is moving that feature into here.

Yes, that was my original PR to fix this problem before I realized I was only treating symptoms. I decided to dive into Multiserver, which I struggled with, and spent dozens of hours trying to fix the stringify() implementation so that we could resolve the root of the problem. When you ask Multiserver to listen on ::, I'd expect it to listen on all addresses. When you ask Multiserver to give you the addresses it's listening on, I'd expect it to give you the addresses it's listening on. That's exactly what this PR does.

(The "scopes" bug that you pointed out is, of course, a bug, and I'd be happy to resolve it if we decide that this branch has a chance of ever being merged.)

I'm leaning towards [throwing on :: and 0.0.0.0].

I understand that, and I hear you, but my question is why? What's the trade-off that we're making? To me, it seems obvious that this module should Just Work when you pass one of those addresses, and I don't see the benefit of trying to emulate my operating system's networking stack in JavaScript.

If your point is that "unspecified addresses are bad and scary and we shouldn't use them" then I might be sympathetic, but from what you've written I'm just seeing "no, we should emulate this behavior in SSB-Config/Secret-Stack instead" and I don't understand which problem that solves or why we should kick this problem up the dependency graph.

[dominictarr]

The network stack will let you bind to all the addresses with :: or 0.0.0.0 but it doesn't have a nice way to tell you what address to use to connect to a given server. But that's what we need. We need not just the ability to bind to an port, but also we need to be able to report the address on which we are reachable.

The worst thing about the network stack is that there isn't any easy way to test all the possibilities, so we have to learn through bitter experience and argue about it. Even the database stuff is easier to test! Many of us have banged our heads against this and if there was an easier way to do it right then we would. It's not our fault that networking is a mess, but we still have to deal with it.

What problem are we solving with throwing on ::? it means that plugins/net doesn't know about scopes, it just listens on an address, and just considers scope to be an opaque grouping. This PR breaks that.

On the other hand, if net understands scopes, it means you can't make up a new scope. But currently, if you wanted to make up a totally new scope name, you can. multiserver doesn't know what a scope is, just what it is.

The automatic binding to 0.0.0.0 is really an instance of the system doing a "helpful" thing that ends up being not that helpful... you end up either redoing it at a higher level, or reversing what it's doing, to make up for the missing part. I'm suggesting redoing :: at a higher level, and you are suggesting reversing it (so we can answer what address it's on).

There are arguments to be made for redoing it: like, maybe you want to bind to local addresses only, and not public (don't make a public server if you the local network happens to give you one). You can't do that with :: but we can if we have control.

[dominictarr]

they way you put it "unspecified addresses are bad" yes that is what I'm saying. When I'm saying we should "emulate this behavior" I just mean that we can't expect ssb users to specify exactly what addresses they are using. We need some way for them to say "bind whatever addresses are available" but I think the systems way of doing that isn't very helpful.

[christianbundy]

Cool, that makes sense to me. If our constraint is "the stringify() function must operate with zero assumptions" then I understand why we'd want to avoid the unspecified addresses.

[christianbundy]

Removing :: from Multiserver feels like a big change, and after this PR I'm really hesitant to invest any time making big changes to Multiserver on my own. Anyone want to work together to come up a plan and make the change? I just bumped into some :: trouble in SSB-WS and it would be great to have this not be in flux.

[christianbundy]

I don't have any more energy to pour into this.