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

Fix 3 misleading .proto TradeInfo field names [#10] #6081

Merged

Conversation

ghubstan
Copy link
Member

Change grpc.proto TradeInfo field names:

  • is_fiat_sent -> is_payment_started_message_sent (payment can be in altcoin)
  • is_fiat_received -> is_payment_received_message_sent (payment can be in altcoin)
  • is_withdrawn -> is_completed (trade can be closed with funds kept in Bisq wallet)

Based on branch 9-use-snake_case-field-names, PR #6080

The new console output formatting (tbl) api is working, and the
deprecated classes just add maintenance work.

Update affected test cases.
Much less ambiguous field value at cost of breaking backward compat in API.
Quite a bit of refactoring.  A lot of number-string formatting was removed.
Rename double marketPriceMargin -> marketPriceMarginPct.
Rename double buyerSecurityDeposit -> buyerSecurityDepositPct.
This was overlooked in tests, but client might use a bad offer-id param.
This protobuf definition and service stub has been in place since
the start of work on the API, but was never fully implmented, nor
intended to be included in the API beta & v1 releases.

Its presence added a useless section to the gRPC API Reference doc.

	https://ghubstan.github.io/slate

Based on branch `7-more-grpcproto-comments`,
PR bisq-network#6068.
The CLI and apitest cases always pass "0", but java & python clients
might pass an empty string. This change avoids number formatting
& scaling problems when clients pass an empty string in the price
parameter.
The client may have passed an empty string for the price parameter,
if only enabling or disabling the offer.  If so, validate with new
price = old price.
Java client code can still use the camelCase field naming convention,
and other language bindings will not be forced to do the same.

Snake_case field names are also recommended in the Protocol Buffers
Sytle Guide:  https://developers.google.com/protocol-buffers/docs/style
- is_fiat_sent		->	is_payment_started_message_sent
	Payment can be in altcoin.
- is_fiat_received	->	is_payment_received_message_sent
	Payment can be in altcoin.
- is_withdrawn		->	is_completed
	Trade can be closed with funds kept in Bisq wallet.
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK - Changes are looking fine 👍

@ripcurlx ripcurlx added this to the v1.8.4 milestone Feb 28, 2022
@ripcurlx ripcurlx merged commit de1c9ba into bisq-network:master Feb 28, 2022
@ghubstan ghubstan deleted the 10-fix-grpcproto-tradeinfo-fieldnames branch March 3, 2022 19:24
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.

2 participants