This is my 3rd attempt to refactor create_transaction(). Perhaps 3rd try is the charm.
For original background motivation and discussion, see previous attempts: #136 and #159.
Some goals of this attempt are:
address Alan's comments in 159.
make the code simpler and cleaner to read, understand, maintain.
make UtxoNotification variants symmetric.
keep create_transaction() simple for caller to use, but also flexible.
lay some groundwork for onchain symmetric key notification
abstract over key/address type in public APIs.
prepare for using derived key indexes in a future commit.
Changes since #159:
highlights / key changes:
input utxos are now automatically matched to the appropriate spending key. previously that was a TODO.
abstract SpendingKey and ReceivingAddr in create_transaction() and all RPC APIs. This should make it possible to add new address types without major refactors post-launch.
add create_raw_transaction() for fine-grained control over inputs and outputs, including change. (Same api exists in bitcoin.)
all create_transaction() related code is now simpler and cleaner, at least to my eye, with significant functionality moved into TxInput, TxInputList, TxOutput, TxOutputList.
create_transaction() return type is now just a Transaction, not a tuple.
change spending key is now a param to create_transaction() in order to keep it &self. (see discussion below).
rpc_server:
own_receiving_address --> next_receiving_address
SpendingKey -> SpendingKeyType in rpc APIs
ReceivingAddress -> ReceivingAddressType in rpc APIs
adapt send_to_many() to create_transaction() changes. (simpler now)
global_state:
simplify TransactionDetails, and is now private struct.
eliminated several private helper fn
simplified create_transaction family of fns with TxInputList, TxOutputList
added doc comments
add generate_tx_outputs() public helper method
create_transaction() args changed:
receiver_data --> tx_outputs
tx_outputs is now &mut, and may have change output appended for caller's use.
add change_spending_key
create_transaction() return type is now just a Transaction, not a tuple.
caller must pass a change spending key into create_transaction() because
next_unused_generation_spending_key() takes &mut self.
added create_raw_transaction() for fine grained control over inputs and outputs
simplify create_transaction() helper fns.
wallet:
add abstract SpendingKeyType and ReceivingKeyType
add wallet_state::find_spending_key_for_utxo()
wallet_state::allocate_sufficient_input_funds_from_lock() now returns
matching SpendingKeyType with each input. (fixes prev TODO)
transaction:
add TxInput, TxInputList, TxOutput, TxOutputList
add UtxoNotifyMethod, UtxoNotification
other:
UtxoReceiver -> TxOutput
Explanations of things that may not be obvious:
preparing for derived keys. Presently some functions call WalletSecret::nth_generation_spending_key(&self, counter: u16), passing 0. I've now added ::next_unused_generation_spending_key(&mut self), which presently always uses index 0 but in the future will increment a stateful counter. The difficulty is that it must take &mut self, so it cannot be called from within GlobalState methods that take &self, such as create_transaction(). So create_transaction() now requires the caller to provide the next unused spending key.
let me know if there's something else. I put a lot of code comments, but let me know if anything is unclear.
Addresses #122. Obsoletes #159.
This is my 3rd attempt to refactor create_transaction(). Perhaps 3rd try is the charm.
For original background motivation and discussion, see previous attempts: #136 and #159.
Some goals of this attempt are:
Changes since #159:
highlights / key changes:
rpc_server:
global_state:
wallet:
transaction:
other:
Explanations of things that may not be obvious:
preparing for derived keys. Presently some functions call
WalletSecret::nth_generation_spending_key(&self, counter: u16)
, passing 0. I've now added::next_unused_generation_spending_key(&mut self)
, which presently always uses index 0 but in the future will increment a stateful counter. The difficulty is that it must take &mut self, so it cannot be called from within GlobalState methods that take &self, such as create_transaction(). So create_transaction() now requires the caller to provide the next unused spending key.let me know if there's something else. I put a lot of code comments, but let me know if anything is unclear.