Add package wtxmgr to use walletdb interface

[tuxcanfly]

NOTE: This pull request requires #147

Moved txstore to legacy and added package wtxmgr which provides walletdb support.

Legacy txstore will be imported as a part of the wallet setup as implemented in #147.

[jrick]

Needs a rebase onto latest intwaddrmgr and some updates for the btcnet -> btcd/chaincfg package move.

[tuxcanfly]

Rebased on top of chaincfg move.

[jrick]

These three changes result in the unlinking of all previous txstore history. Instead, I would do it this way:

git mv txstore legacy/txstore # move old txstore to legacy
# rewrite txstore imports to point to legacy so everything continues to build
git commit -a -m "Move txstore package to legacy directory."

# create new wtxmgr package.  Probably easiest to just copy the same files as in your commits.
git add wtxmgr/*
git commit -a -m "Add pkg wtxmgr"

# Rewrite all "legacy/txstore" imports to point to the new wtxmgr package and finish integration.
git commit -a -m "Integrate wtxmgr package."

If done this way, each commit will continue to build (important for bisect) and we still keep all history of how the old txstore package was created by looking at files in the legacy directory (without needing to checkout an older commit).

[davecgh]

I agree with @jrick here. That is how I did the waddrmgr as well.

[tuxcanfly]

Thanks, good idea to preserve history, will follow that.

[jrick]

I realize this is more of a txstore conversion to wallet db, but one of the changes I want is to remove the block index. We won't have this information available when wallet gains SPV support, but txstore/wtxmgr relies on it for correctness (as a lookup key).

We should either bite the bullet and rip it out now, or do it in a db upgrade later.

[jrick]

In no particular order:

• Several cases of unneeded branching for final returns.

// Instead of this:
if err := fn(); err != nil {
    return err
}
return nil

// Prefer this:
return fn()

• Slices are created based on a size in the metadata and then each index is set by iterating over values in buckets. This can result in an OOB panic if the db is corrupt or an incorrect size was previously added. Use append here instead (or check that no more than n values are read from the bucket).
• Keep function signatures on a single line, even if they go over some magic number of characters.
• When incrementing counts in the metadata, use n+1 (a temporary) at the call site instead of incrementing the variable and writing the new number, if n is not used again later (I haven't yet seen any places where it was).
• Be cautious with allocations. They can become very expensive, especially in a GC'd language. This code currently creates a lot of garbage during serialization and deserialization. I'll work up some better ways to do this if you don't, but it's a good habit to cringe a little each time you know an allocation is happening and find a more efficient way to do the same thing. For this code, the key will be buffer reuse.
• Documentation (doc.go) needs to be updated (some descriptions are specific to the old txstore).
• I am in favor of using tools like stringer to automatically produce fmt.Stringer implementations with generated code since it means less boilerplate code to write and maintain (for example, the ErrorCode type could use this).
• I don't think this DB format makes as much sense in an actual DB as it did when it was all in memory and a single flat file. For example, the one thing that really stands out to me is how every transaction record must be replaced any time any changes are made to it (e.g. adding Debits or Credits, or marking a Credit spent). Not only that, but the Record also includes the entire wire.MsgTx, so there is a lot of unnecessary serialization and I/O going on whenever something changes. I think having a dedicated bucket just for raw transactions is a better solution, and additional details about credits and debits can be stored in another bucket.

I'll continue with the review later but those are some things to get you started on.

[tuxcanfly]

Thanks for the detailed points, will work on those and keep them in mind for future.

[davecgh]

I agree with all of @jrick's points save the stringer. I do agree that in general using go generate is nice, but that particular tool generates code that I don't consider anywhere near the quality it needs to be. It doesn't follow Go standards at all, and as a result, causes tools like golint to fail. As long as that remains the case, I don't want code from stringer anywhere near our repos.

[tuxcanfly]

@jrick Updated as per yesterday's discussion i.e. buffer reuse + object reuse, docs and style.

Also started with benchmarks for serialization which might be useful soon.