Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use wtxmgr for input selection. #130

Merged
merged 17 commits into from
Mar 25, 2016
Merged

Use wtxmgr for input selection. #130

merged 17 commits into from
Mar 25, 2016

Conversation

jrick
Copy link
Member

@jrick jrick commented Mar 24, 2016

Note: Depends on #127.

This adds a new wtxmgr API for resumable, incremental input selection
that is performed under a single database transaction, and hooks up
the API for transaction creation in the wallet package. The new
wtxmgr API is compatible with the API required by the txauthor package
(txauthor.InputSource), but requires an (implicit) type conversion so
the wtxmgr package doesn't introduce an unnecessary dependency on the
txauthor package.

Stake and multisig transaction creation is unmodified by this change.

jrick and others added 16 commits February 28, 2016 22:22
StartBtcdRpc becomes StartConsensusRpc.

This is useful for forks such as decred or if someone were to write
another compatible server.

Bump up the api version as this is a change.
This began as a change to improve the fee calculation code and evolved
into a much larger refactor which improves the readability and
modularity of all of the transaction creation code.

Transaction fee calculations have been switched from full increments
of the relay fee to a proportion based on the transaction size.  This
means that for a relay fee of 1e3 satoshis/kB, a 500 byte transaction
is only required to pay a 5e2 satoshi fee and a 1500 byte transaction
only need pay a 1.5e3 fee.  The previous code would end up estimating
these fees to be 1e3 and 2e3 respectively.

Because the previous code would add more fee than needed in almost
every case, the transaction size estimations were optimistic
(best/smallest case) and signing was done in a loop where the fee was
incremented by the relay fee again each time the actual size of the
signed transaction rendered the fee too low.  This has switched to
using worst case transaction size estimates rather than best case, and
signing is only performed once.

Transaction input signature creation has switched from using
txscript.SignatureScript to txscript.SignTxOutput.  The new API is
able to redeem outputs other than just P2PKH, so the previous
restrictions about P2SH outputs being unspendable (except through the
signrawtransaction RPC) no longer hold.

Several new public packages have been added:

wallet/txauthor - transaction authoring and signing
wallet/txfees - fee estimations and change output inclusion
wallet/txrules - simple consensus and mempool policy rule checks

Along with some internal packages:

wallet/internal/txsizes - transaction size estimation
internal/helpers - context free convenience functions

The txsizes package is internal as the estimations it provides are
specific for the algorithms used by these new packages.
This prevents races when setting a new relay fee through the legacy
RPC server (settxfee).

Fixes btcsuite#379.
This corrects and simplifies the shutdown logic for interrupts, the
walletrpc.WalletLoaderService/CloseWallet RPC, and the legacy stop RPC
by both stopping all wallet processes and closing the wallet database.
It appears that this behavior broke as part of the wallet package
refactor, causing occasional nil pointer panics and memory faults when
closing the wallet database with active transactions.

Fixes btcsuite#282.

Fixes btcsuite#283.
Note that this is a breaking change since it removes the mainnet
config option, replacing it with a testnet option.  Old configuration
files that set mainnet=1 will cause the wallet to error during startup
since extraneous flags are treated as errors.

Because configuration files will have to be updated for the change
regardless, the old deprecated (and unused) options `disallowfree` and
`keypoolsize` have also been removed.

Closes btcsuite#383.
This change only prevents creating new accounts with the empty name or
renaming an existing account to one.  Any accounts in the DB that are
already named the empty string are left untouched (and should be
renamed to something meaningful by the user).

Fixes btcsuite#369.
These notifications were added to support real time updates for
btcgui.  As the btcgui project is no longer being developed, there are
no more consumers of this API, and it makes sense to remove them given
their various issues (the largest being that notifiations are sent
unsubscribed to clients that may never be interrested in them).

A new notification server has already been added to the wallet package
to handle notifications in a RPC-server agnostic way.  This server is
the means by which the wallet notifies changes for gRPC clients.  If
per-client registered notifications are to be re-added for the
JSON-RPC server, they should be integrated with the new notification
server rather than using this legacy code.
This makes btcwallet match btcd's behavior.

Initially pointed out in:
decred/dcrwallet#125
This changes the wallet.Open function signature to remove the database
namespace parameters.  This is done so that the wallet package itself
is responsible for the location and opening of these namespaces from
the database, rather than requiring the caller to open these ahead of
time.

A new wallet.Create function has also been added.  This function
initializes a new wallet in an empty database, using the same
namespaces as wallet.Open will eventually use.  This relieves the
caller from needing to manage wallet database namespaces explicitly.

Fixes btcsuite#397.
Conflicts:
	README.md
	chain/chain.go
	cmd/dropwtxmgr/main.go
	config.go
	dcrwallet.go
	internal/legacy/keystore/keystore.go
	internal/legacy/keystore/keystore_test.go
	internal/legacy/rename/rename_unix.go
	internal/rpchelp/genrpcserverhelp.go
	internal/rpchelp/helpdescs_en_US.go
	internal/rpchelp/methods.go
	internal/zero/array.go
	internal/zero/benchmark_test.go
	internal/zero/slice.go
	internal/zero/slice14.go
	internal/zero/zero_test.go
	log.go
	netparams/params.go
	params.go
	rpc/documentation/README.md
	rpc/documentation/api.md
	rpc/legacyrpc/errors.go
	rpc/legacyrpc/methods.go
	rpc/legacyrpc/rpcserver_test.go
	rpc/legacyrpc/server.go
	rpcserver.go
	sample-dcrwallet.conf
	version.go
	votingpool/common_test.go
	votingpool/db.go
	votingpool/db_wb_test.go
	votingpool/doc.go
	votingpool/error.go
	votingpool/error_test.go
	votingpool/example_test.go
	votingpool/factory_test.go
	votingpool/input_selection_wb_test.go
	votingpool/internal_test.go
	votingpool/pool.go
	votingpool/pool_test.go
	votingpool/pool_wb_test.go
	votingpool/test_data_test.go
	votingpool/withdrawal.go
	votingpool/withdrawal_test.go
	votingpool/withdrawal_wb_test.go
	waddrmgr/address.go
	waddrmgr/common_test.go
	waddrmgr/db.go
	waddrmgr/doc.go
	waddrmgr/error.go
	waddrmgr/error_test.go
	waddrmgr/internal_test.go
	waddrmgr/manager.go
	waddrmgr/manager_test.go
	waddrmgr/sync.go
	wallet/chainntfns.go
	wallet/createtx.go
	wallet/createtx_test.go
	wallet/createtx_test_disabled.go
	wallet/doc.go
	wallet/loader.go
	wallet/notifications.go
	wallet/rescan.go
	wallet/wallet.go
	walletdb/bdb/db.go
	walletdb/bdb/doc.go
	walletdb/bdb/driver.go
	walletdb/bdb/driver_test.go
	walletdb/bdb/interface_test.go
	walletdb/db_test.go
	walletdb/doc.go
	walletdb/example_test.go
	walletdb/interface.go
	walletdb/interface_test.go
	walletsetup.go
	wtxmgr/db.go
	wtxmgr/doc.go
	wtxmgr/example_test.go
	wtxmgr/query.go
	wtxmgr/query_test.go
	wtxmgr/tx.go
	wtxmgr/tx_test.go
	wtxmgr/unconfirmed.go
@jcvernaleo
Copy link
Member

This passes my tests and looks fine to me.
Needs a rebase then OK from me.

This adds a new wtxmgr API for resumable, incremental input selection
that is performed under a single database transaction, and hooks up
the API for transaction creation in the wallet package.  The new
wtxmgr API is compatible with the API required by the txauthor package
(txauthor.InputSource), but requires an (implicit) type conversion so
the wtxmgr package doesn't introduce an unnecessary dependency on the
txauthor package.

Stake and multisig transaction creation is unmodified by this change.
@jrick
Copy link
Member Author

jrick commented Mar 24, 2016

Squashed the fix

@jcvernaleo
Copy link
Member

Yeah, I meant squash, not rebase but I see you figured that out.

@cjepson
Copy link
Contributor

cjepson commented Mar 25, 2016

tACK, still hit a bad tx with priority but that's unhandled in btcwallet I think. Works OK on simulation network.

@jcvernaleo jcvernaleo merged commit b0aff95 into decred:master Mar 25, 2016
@jrick jrick deleted the inputsource branch March 25, 2016 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants