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

Refactor BtcWalletService to let api override fee rates #4896

Conversation

ghubstan
Copy link
Contributor

@ghubstan ghubstan commented Dec 4, 2020

BtcWalletService was changed to allow the api to override tx fee rates from the sendbsq and sendbtc methods. The api methods will still be able to use the network fee service and custom tx fee rate preference, and set / unset the custom tx fee rate preference, but the change will permit the addition of an optional txFeeRate parameter to the sendbsq and sendbtc methods (todo). A few other minor changes (style and removal of never thrown ex spec) were also made to this class.

Two BtcWalletService methods were refactored.

  • The redundant (was always true) boolean isSendTx argument was removed from the completePreparedVoteRevealTx method signature.

  • The redundant (was always true) boolean useCustomTxFee was removed from the completePreparedBsqTx method signature.

  • The completePreparedSendBsqTx method was overloaded with a 2nd parameter (Coin txFeePerVbyte) to allow api to override fee service and custom tx fee rate when sending BSQ or BTC.

  • The completePreparedBsqTx method was overloaded with a 3rd parameter (Coin txFeePerVbyte) to allow api to override fee service and custom tx fee rate when sending BSQ or BTC.

The following line was deleted from the completePreparedBsqTx method because txFeePerVbyte is now an argument:

`Coin txFeePerVbyte = useCustomTxFee ? getTxFeeForWithdrawalPerVbyte() : feeService.getTxFeePerVbyte();`

This useCustomTxFee value was always true, and redundant here because getTxFeeForWithdrawalPerVbyte() returns feeService.getTxFeePerVbyte() or the custom fee rate preference. i.e.,

Coin txFeePerVbyte = useCustomTxFee ? getTxFeeForWithdrawalPerVbyte() : feeService.getTxFeePerVbyte();
is equivalent to
Coin txFeePerVbyte = getTxFeeForWithdrawalPerVbyte();

LockupTxService, UnlockTxService, BsqSendView, and BsqTransferService were adjusted to the BtcWalletService refactoring.

This is the 2nd in a chain of PRs beginning with #4884.
PR #4884 should be reviewed before this one.

BtcWalletService was changed to allow the api to override tx fee
rates from the sendbsq and sendbtc methods.  The api methods will
still be able to use the network fee service and custom tx fee rate
preference, and set / unset the custom tx fee rate preference, but
the change will permit the addition of an optional txFeeRate parameter
to the sendbsq and sendbtc methods (todo).  A few other minor changes
(style and removal of never thrown ex spec) were also made to this class.

Two BtcWalletService methods were refactored.

- The redundant (was always true) boolean isSendTx argument was removed
  from the completePreparedVoteRevealTx method signature.

- The redundant (was always true) boolean useCustomTxFee was removed
  from the completePreparedBsqTx method signature.

- The completePreparedSendBsqTx method was overloaded with a 2nd parameter
  (Coin txFeePerVbyte) to allow api to override fee service and custom
  tx fee rate when sending BSQ or BTC.

- The completePreparedBsqTx method was overloaded with a 3rd parameter
  (Coin txFeePerVbyte) to allow api to override fee service and custom
  tx fee rate when sending BSQ or BTC.

The following line was deleted from the completePreparedBsqTx method
because txFeePerVbyte is now an argument:

	Coin txFeePerVbyte = useCustomTxFee ? getTxFeeForWithdrawalPerVbyte() : feeService.getTxFeePerVbyte();

This useCustomTxFee value was always true, and redudant here because
getTxFeeForWithdrawalPerVbyte() returns feeService.getTxFeePerVbyte()
or the custom fee rate preference. i.e.,

Coin txFeePerVbyte = useCustomTxFee ? getTxFeeForWithdrawalPerVbyte() : feeService.getTxFeePerVbyte();

	is equivalent to

Coin txFeePerVbyte = getTxFeeForWithdrawalPerVbyte();

LockupTxService, UnlockTxService, BsqSendView, and BsqTransferService
were adjusted to this BtcWalletService refactoring.
@ghubstan
Copy link
Contributor Author

ghubstan commented Dec 5, 2020

force travis build after jar download failed

@ghubstan ghubstan closed this Dec 5, 2020
@ghubstan ghubstan reopened this Dec 5, 2020
Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the changes look ok.

Needs review from @ManfredKarrer

@ripcurlx ripcurlx added the in:api label Dec 9, 2020
Copy link
Contributor

@chimp1984 chimp1984 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

Copy link
Contributor

@ManfredKarrer ManfredKarrer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@ghubstan
Copy link
Contributor Author

Review and merged with #4966.

@ghubstan ghubstan closed this Dec 23, 2020
@ghubstan ghubstan deleted the 02-refactor-completePreparedSendBsqTx branch December 23, 2020 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants