Proposal: Remove dependency on NebulousLabs/Sia

[lukechampine]

us does not strongly depend on the NebulousLabs/Sia packages; most of the references are to small types like modules.NetAddress, types.BlockHeight, etc. However, even a single reference is sufficient to drag in the entire repo as a dependency, which significantly bloats the size of binaries that import us packages. This also impacts the us bindings: the resulting ObjC framework, for example, is 53 MB, which is just crazy.

Unfortunately, there are a few places where the dependency is hard to remove. In particular, wallet.ChainScanner needs to implement modules.ConsensusSetSubscriber, which means it must depend on the modules.ConsensusChange type. Additionally, if we substitute our own types for NebulousLabs/Sia types, it becomes harder to write code that interoperates with both us and NebulousLabs/Sia.

I propose that we make two changes:

• Replace all types/functions imported from NebulousLabs/Sia with local types/functions.
• Add a new interop package to us-bindings that converts between us types and NebulousLabs/Sia types.

To solve the wallet.ChainScanner problem, we would redefine the method to depend on a locally-defined copy of modules.ConsensusChange, and then call interop.ToConsensusSetSubscriber on it to make it satisfy modules.ConsensusSetSubscriber. This is viable because we only need to actually subscribe to the consensus set in binaries like walrus and during certain tests, so you only "pay for" the NebulousLabs/Sia dependency when you actually need it. Accordingly, the bindings should shrink enormously.

[lukechampine]

Follow up: I wasn't 100% sure that NebulousLabs/Sia was responsible for the large binaries, so I fact-checked myself using the goweight tool. This tool indicates that indeed, a sizeable percentage of a binary like user comes from NebulousLabs packages.

However, when I tried another tool, it indicated that NebulousLabs packages are only responsible for about 1.6% of the compiled binary.

The only way to truly be sure is to strip out the dependency, recompile, and see how much smaller the binary is. Unfortunately, that requires a fair amount of work.

Still, even if binaries don't shrink as much as I'd hoped, there are other reasons why removing the dependency is desirable. In particular, it drags a ton of packages into our go.sum, which makes a proper audit far more difficult. See the attached graph; if not for NebulousLabs/Sia, we would depend on a total of 5 non-stdlib packages (pkg/errors, aead/chacha20, bbolt, x/crypto, x/sys):

Of those, pkg/errors could be replaced with the new Go 1.13 wrapped errors, and aead/chacha20 could be replaced with x/crypto once golang/crypto#108 is merged. bbolt could also conceivably be moved to a separate package, although that's probably overkill. At any rate, it would be nice to have only three non-stdlib dependencies.

[jkawamoto]

This is a significant issue for us, too. The problem is NebulousLabs/Sia tends to print a stack trace instead of panicking when some critical errors occur. For example, we got this:

goroutine 7358 [running]:
runtime/debug.Stack(0x44ff77, 0x0, 0xc000b5b268)
        /usr/local/Cellar/go/1.14.2_1/libexec/src/runtime/debug/stack.go:24 +0x9d
runtime/debug.PrintStack()
        /usr/local/Cellar/go/1.14.2_1/libexec/src/runtime/debug/stack.go:16 +0x22
gitlab.com/NebulousLabs/Sia/build.Critical(0xc000b5b310, 0x1, 0x1)
        /Users/junpei/pkg/mod/gitlab.com/!nebulous!labs/[email protected]/build/critical.go:16 +0xaa
gitlab.com/NebulousLabs/Sia/types.Currency.Sub(0xc000b5bc00, 0xc000bc3680, 0x2, 0x7, 0x100, 0xc000bc3700, 0x2, 0x7, 0x0, 0x0, ...)
        /Users/junpei/pkg/mod/gitlab.com/!nebulous!labs/[email protected]/types/currency.go:190 +0x14e
lukechampine.com/us/renter/proto.(*Session).RenewAndClearContract(0xc0000ba580, 0x1191000, 0xc0004c8258, 0x117f940, 0xc0004c8258, 0xc0004f4400, 0xc000aae600, 0x2, 0x7, 0x3fbe5, ...)
        /Users/junpei/pkg/mod/lukechampine.com/[email protected]/renter/proto/renew.go:289 +0xa82
lukechampine.com/us/renter/proto.RenewContract(0x1191000, 0xc0004c8258, 0x117f940, 0xc0004c8258, 0xa6d93141424e68de, 0x9d33a41fb271994, 0xe66fb7c5713620c8, 0x7b2dc55531564d00, 0xc0004f44b0, 0x40, ...)
        /Users/junpei/pkg/mod/lukechampine.com/[email protected]/renter/proto/renew.go:31 +0x271
...
Critical error: negative currency not allowed
Please submit a bug report here: https://gitlab.com/NebulousLabs/Sia/issues

This behaviour doesn't work for us because of these reasons:

• "Please submit a bug report here: https://gitlab.com/NebulousLabs/Sia/issues" doesn't make sence for any users since our app doesn't use said;
• our app cannot know there was a problem since build.Critical (and thus proto.RenewContract) don't return any error, nor do it panic;
• printing stack traces breaks our log parser.

Although we can customize this build package's behaviour with build tags, it requires to set debug,testing to panic instead of printing stack traces. However, those tags will break other packages for production.

[lukechampine]

In fairness, anything that triggers a build.Critical is a developer error that I should fix immediately. But yeah, I would use a normal panic for that.