Make IPLD plugin a package, not a sub-module

[Wondertan]

The PR:

• Actually makes IPLD plugin a package (closes: ipld: make sub-module a package instead #291)
• Updates plugin code to conform gofmt and golangci-lint`
• Changes NMTNodeAdder to catch possible errors when plugin nodes are added.
• Updates .so plugin building for IPFS in Makefile

[Wondertan]

So it seems like both go fmt and golangci-lint run were ignoring that code...

[liamsi]

So it seems like both go fmt and golangci-lint run were ignoring that code...

Ouch, ouch. Can you fix those lints here too?

[Wondertan]

@liamsi, golangcilint opened an issue about not checking errors when we add nodes to Batch here. In my opinion, this could silently produce undefined behaviors, so I would fix this now, rather than debug myself to death when the unobvious bug appears, as an error was silently ignored. However, it is not obvious how to handle errors here. Options:

• Extend VisitorFunc and tree.Root to return errors. In general, Visitors can do some funky stuff that can return errors, so adding optional erroring would enhance NMT API IMO.
• Introduce another Visitor that would be called on Push, though that does not work for intermediate nodes.
• Update NmtNodeAdder struct to have an error, similar to Batch and introduce Commit method that would wrap Batch.Commit, check for errors if any, and be used here.

[liamsi]

Cool, that you wrote up different alternatives 👌🏼

Update NmtNodeAdder struct to have an error, similar to Batch and introduce Commit method that would wrap Batch.Commit, check for errors if any, and be used here.

I think that is the best approach.

Pros:

• least invasive and only adds errors / error-handling where it's necessary; in other words very local change
• follows an already well-established pattern of the used underlying data structure (Batch)

Cons:

• not generalizable to other cases (no a biggie as I don't see us adding a bunch of erroring funky visitors)

Regarding the other two:

• Extend VisitorFunc and tree.Root to return errors. In general, Visitors can do some funky stuff that can return errors, so adding optional erroring would enhance NMT API IMO.
• Introduce another Visitor that would be called on Push, though that does not work for intermediate nodes.

I'm not really sure the second would work and the first requires changing the tree interface which we should avoid (at least in this PR imo).

[codecov-commenter]

Codecov Report

Merging #294 (c8aea32) into master (93c88a1) will decrease coverage by 0.42%.
The diff coverage is 59.80%.

@@            Coverage Diff             @@
##           master     #294      +/-   ##
==========================================
- Coverage   62.27%   61.85%   -0.43%     
==========================================
  Files         260      270      +10     
  Lines       23218    23890     +672     
==========================================
+ Hits        14460    14776     +316     
- Misses       7265     7578     +313     
- Partials     1493     1536      +43     

Impacted Files	Coverage Δ
cmd/tendermint/commands/light.go	16.66% <0.00%> (ø)
cmd/tendermint/commands/run_node.go	0.00% <ø> (ø)
config/config.go	77.74% <ø> (-0.15%)	⬇️
config/toml.go	60.86% <ø> (ø)
consensus/replay_file.go	0.00% <0.00%> (ø)
consensus/state.go	67.52% <0.00%> (+0.27%)	⬆️
crypto/merkle/proof_value.go	0.00% <0.00%> (ø)
crypto/random.go	50.00% <ø> (ø)
evidence/pool.go	65.35% <ø> (ø)
libs/db/memdb/batch.go	0.00% <0.00%> (ø)
... and 61 more

[liamsi]

The e2e tests fail because of https://github.com/lazyledger/lazyledger-core/blob/874df76f139b4a3effcd2a74d6078c9d08b6d1ba/test/e2e/docker/Dockerfile#L16

[liamsi]

This looks great! Thanks @Wondertan

Left a minor nit regarding a private function.