jrick
commented
6 years ago walletdb is an interesting package to deal with as well. It (specifically, the walletdb.DB type) is exported by the wallet package so that wallets can be created in, written to, and opened from a database. Because the callers must handle the DB, this package can not simply be made internal. However, all of the methods of the walletdb.DB interface do not need to be available to consumers of the module; only the wallet (and udb) packages need these methods.
There are two possible improvements I see here. Either the walletdb.DB type can become an opaque type that can refer to a database, and provides the wallet and udb packages with the methods needed, but without exposing those methods to consumers of the module. A second way (which may not work, and needs further investigation) could be to refer to the database by the filename rather than the database object itself.
kLkA
dcrwallet is not just a program -- like dcrd, it also includes packages that can be directly imported by other tools for integrating various wallet functionality. For example,
lndimports thewalletpackage in order to implement key storage and signing. However, not all packages are designed for external consumption, and there should be a clear distinction between which packages must follow strict semantic versioning and those which don't. This issue provides a discussion to determine which packages should be created into modules, which should not, and whether the package layout must adjust to accommodate.The following are all of the packages imported or used by the main package, and the repo's root main package itself (all of which make up the
dcrwalletexecutable):Some of my own observations/thoughts/rambling follow. Feel free to follow up with comments whether you agree, disagree, or have suggestions. Once thoughts are fleshed out, a new repo/module layout can be proposed.
The repository currently makes use of
internalpackages. These are packages which can not be imported by any other packages without the same package prefix (up to the/internal). If they are included in a module, they do not make up the module's external API and compatibility may be broken without creating a new major version of the module. It can be advantageous to increase the number of internal packages in order to reduce a module's exported API.Some packages must be available for consumption by other projects. I believe this must include (at minimum)
errors,pgpwordlistandwalletseed,version,wallet,wallet/txauthor,wallet/txrules,wallet/internal/txsizes, andchain. Rationale for each follows:errorsimplements error handling throughout the project, and errors returned by modules must be compared using this package.pgpwordlistandwalletseedare each small packages and together implement encoding and decoding wallet seeds in hex or PGP word list format. They do not depend on any other wallet packages (besideserrors) and would be trivial to make modules with semver guarantees.versionimplements the release version of thedcrwalletmain package. This is useful for build systems which must include the release version of dcrwallet, as storing this information in the main package would prevent it from being imported. While it may be useful for external consumption, as it describes thedcrwalletversion, perhaps it should remain under the root module.walletis easily the most interesting package for external projects. It provides nearly all of the essential wallet functionality (managing transactions, authoring transactions, addresses, keys, signing, etc.). I say nearly all functionality as this package does not provide, by itself, any way to interact with the Decred network. That functionality is currently provided by awallet.NetworkBackendinterface which is implemented bychain, and soon to be implemented by thespvpackage as well.walletalso exposes one of the largest package APIs in the project, and I expect that providing strict semver backwards compatibility may result in many major version bumps as the API is cleared of years of unfortunate decisions.wallet/txauthorandwallet/txrules, while under thewalletpackage path, are each standalone packages which may prove useful to other projects.txauthorprovides a method of creating unsigned transactions over any UTXO set, andtxrulesdescribes basic consensus and policy rules, such as the transaction relay fee and dust threshold.wallet/internal/txsizesis not currently available to be imported by external projects, but this would be a good opportunity to make it so. The package implements estimation calculations of transaction sizes given various numbers of inputs and outputs. It is currently an internal package since it makes assumptions regarding the precise types of inputs being used by the wallet to construct the transaction, and therefore is not suitable for all transaction size estimates. However, it is still useful, and as long as the API is modified to reflect its shortcomings, it can be exported as or as part of a module.chain(andspvin the future) provide an implementation ofwallet.NetworkBackendand implement the ability to keep a wallet in sync with the Decred network. This is desirable for external projects if they need to care about more than just offline signing, such as details about the current blockchain, wallet UTXO set, transactions, double spend detection, and more.Some external projects may want to depend on packages in submodules not in the above list. I currently see this group made up of packages such as
ticketbuyerandloader. I'm open for future support for making modules out of these packages, but do not deem it a priority until the other packages have been modularized. Both packages depend heavily on the wallet and network abstractions. I would especially like to makeloadera public module off the bat, as it provides a simpler way to manage the lifetime of a wallet, but this package exportsticketbuyerand should wait untilticketbuyerbecomes a module.There are packages which likely should be internal packages but are not, either due to accident or necessity. For example, the
github.com/decred/dcrwallet/wallet/udbpackage provides the implementation for storing and retrieving all database data for thegithub.com/decred/dcrwallet/walletpackage. This is an internal implementation detail, andudbshould not be available to import by other projects. At the moment, there exists a layering violation whereudbtypes are exported by thewalletpackage and consumed by other packages such as the RPC servers, which is whyudbis currently not an internal package.snaclis another good candidate to make internal, and would be easier to do thanudbas it is not exported by any other proposed module. In addition to these, there are other packages, such as the service implementations of the RPC servers, which should not be imported by external projects as they are only used by the root module, but it is unclear to me at this point whether these should be made internal packages of the root module or left alone.