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

Add new api method gettransaction #4948

Merged
merged 22 commits into from
Dec 23, 2020
Merged

Conversation

ghubstan
Copy link
Member

This change was prompted by recent changes in the main branch to allow a tx memo field to be set from the UI and API.

This and the prior PR address the API's need to be able to fetch a tx (with a memo). The API can now get a completed trade's withdrawalTxId and use it as a gettransaction parameter.

A summary of changes by file:

grpc.proto

  • Added withdrawalTxId field to existing TradeInfo proto & wrapper.
  • Reordered fields in TradeInfo proto.
  • Added new fields to be displayed by TxInfo proto in CLI.
  • Fixed typo: unsetTxFeeRatePreference -> UnsetTxFeeRatePreference.
  • Added new GetTransaction rpc.

GrpcWalletsService - Added new getTransaction gRPC boilerplate.

CoreWalletsService - Added new getTransaction implementation.

TxInfo - Added the new fields for displaying a tx summary in the CLI. This is not intended to be more than a brief summary; a block explorer or bitcoin-core client should be used to see the complete definition.

TradeInfo - Added the new withdrawalTxId field defined in grpc.proto.

CliMain - Added new case gettransaction.

TransactionFormat - Formats a TxInfo sent from the server to CLI.

ColumnHeaderConstants - Added console headers used by TransactionFormat.

TradeFormat - Displays a completed trade's withdrawalTxId if present.

apitest - Adjusted affected tests: assert tx memo is persisted and test gettransaction.

This is the 8th in a chain of PRs beginning with #4884.
PR #4937 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.
If present in the sendbsq command, the parameter will override the fee
service and custom fee rate setting for the BSQ transaction.

Also changed the sendbsq grpc return type to a lightweight TX proto wrapper.

Besides some small refactoring in the CLI, all the changes are
adjustments for this new sendbsq parameter and its new grpc return value.
Takes an address, amount, and optional txfeerate param,
returns a lightweight TxInfo proto.

Also overloaded two BtcWalletService methods to allow sendbtc
to pass in the tx fee rate -- overriding the fee service and
custom fee rate setting.
- Added optional memo parameter to the api's sendbtc and
  withdrawfunds commands.

- Removed the @nullable annotation was removed because protobuf
  does not support null.

- Visibility in two wallet check methods were changed from private
  to pkg protected so the CoreTradeService could use them.

- Adjusted affected tests.  (Asserting the memo field was set on a
  transaction cannot be checked from apitest yet.)
The withdrawalTxId field will be set in TradeManager#onWithdrawRequest
upon successful trade completion and withdrawal of funds.

Persisting withdrawalTxId will allow the api and ui to find the withdrawalTxId
for a completed trade after the seller withdraws funds to an external wallet.
In turn, the withdrawal tx's memo field will be accessible in a new (todo)
api getTx(txID) api method.

Changed:

- Appended field 'string withdrawal_tx_id = 40' to pb.proto's Trade message.

- Added nullable 'String withdrawalTxId' to Trade entity class.

- Added trade.setWithdrawalTxId(transaction.getTxId().toString()) in
  TradeManager#onWithdrawRequest's callback.
This change was prompted by the recent changes in the main branch to
allow a tx memo field to be set from the UI and API.

This and the prior PR address the API's need to be able to fetch a
tx (with a memo).  The API can now get a completed trade's withdrawal
txid and pass it as a gettransaction parameter.

See previous PR "Append nullable withdrawalTxId field to Trade".

	bisq-network#4937

A summary of changes by file:

grpc.proto

- Added withdrawalTxId field to existing TradeInfo proto & wrapper.
- Reordered fields in TradeInfo proto.
- Added new fields to be displayed by TxInfo proto in CLI.
- Fixed typo: unsetTxFeeRatePreference -> UnsetTxFeeRatePreference.
- Added new GetTransaction rpc.

GrpcWalletsService - Added new getTransaction gRPC boilerplate.

CoreWalletsService - Added new getTransaction implementation.

TxInfo - Added the new fields for displaying a tx summary from CLI.
This is not intended to be more than a brief summary;  a block explorer
or bitcoin-core client should be used to see the complete definition.

TradeInfo - Added the new withdrawalTxId field defined in grpc.proto.

CliMain - Added new 'case gettransaction'.

TransactionFormat - Formats a TxInfo sent from the server to CLI.

ColumnHeaderConstants - Added console headers used by TransactionFormat.

TradeFormat - Displays a completed trade's WithdrawalTxId if present.

Apitest - Adjusted affected tests: assert tx memo is persisted and
test gettransaction.
As per commit 88f26f9,
do not autofill all currencies by default but keep all unselected.
@sqrrm sqrrm merged commit 8ee3a15 into bisq-network:master Dec 23, 2020
@ghubstan ghubstan deleted the 08-gettransaction branch December 23, 2020 18:59
@ripcurlx ripcurlx added this to the v1.5.3 milestone Dec 27, 2020
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.

3 participants