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

Enhancement of listtrades #1232

Closed
kilrau opened this issue Sep 16, 2019 · 3 comments · Fixed by #1567
Closed

Enhancement of listtrades #1232

kilrau opened this issue Sep 16, 2019 · 3 comments · Fixed by #1567
Assignees
Labels
enhancement New feature or request has PR issues with an open PR P2 mid priority

Comments

@kilrau
Copy link
Contributor

kilrau commented Sep 16, 2019

  • Completed at: Timestamp (formatted, json can be unix)
  • Order ID: UUID of the original order, can be the OrderID of multiple trades in the event of partial fills. abbrev. in formatted output, full in json output.
  • Trade ID: rhash , abbreviated in formatted output, full in json output
  • Price per ETH: 0.02 BTC
  • Buy (Received): 50 ETH
  • Sell (Sent): 1 BTC
  • Add. Network Fees: 0.0000004 BTC
  • Role: Taker/Maker
  • Peer: Alias

And probably we can close #625, I see a huge overlap and would want to avoid confusion by having two very similar calls listing trade/swap history.

@kilrau kilrau added the enhancement New feature or request label Sep 16, 2019
rsercano added a commit that referenced this issue Sep 24, 2019
@rsercano rsercano added the has PR issues with an open PR label Sep 27, 2019
@kilrau kilrau added the P1 top priority label Oct 9, 2019
@kilrau kilrau added P2 mid priority and removed P1 top priority labels Jan 15, 2020
@kilrau
Copy link
Contributor Author

kilrau commented May 19, 2020

Took #1243 (comment) and changed Date -> Timestamp

Trading Pair
Amount
Side
Price

condensed this into (imho better readable):

* `Price per ETH`: `0.02 BTC`
* `Buy (Received)`: `50 ETH`
* `Sell (Sent)`: `1 BTC`

Not necessary imo:

Total payment
Order Type (Limit / Market)
Local ID

@kilrau
Copy link
Contributor Author

kilrau commented May 19, 2020

I thought to leave routing fees out of the buy/sell amount to keep them clean and comparable among peers. That's why Add. Do you think that's clear?

Also I thought about renaming Add. Routing Fees to Add. Network Fees since there will be traders who don't know what and why routing fees and I think there will be different fees, e.g. for collateralization, new channels, inbound liquidity and not just routing fees. So I thought Network Fees is more generic and easier to digest for most. Let me know what you think.

@sangaman
Copy link
Collaborator

So I thought Network Fees is more generic and easier to digest for most. Let me know what you think.

I like this naming, but before we can display it we need to preserve it. I'm tracking it in #1565.

sangaman added a commit that referenced this issue May 20, 2020
This replaces the partially implemented `ListTrades` call with the more
comprehensive and descriptive `TradeHistory`. This displays key info
about every trade we have made.

Closes #1232.
sangaman added a commit that referenced this issue May 20, 2020
This replaces the partially implemented `ListTrades` call with the more
comprehensive and descriptive `TradeHistory`. This displays key info
about every trade we have made.

Closes #1232.
sangaman added a commit that referenced this issue May 22, 2020
This replaces the partially implemented `ListTrades` call with the more
comprehensive and descriptive `TradeHistory`. This displays key info
about every trade we have made.

Closes #1232.
sangaman added a commit that referenced this issue May 27, 2020
This replaces the partially implemented `ListTrades` call with the more
comprehensive and descriptive `TradeHistory`. This displays key info
about every trade we have made.

Closes #1232.
sangaman added a commit that referenced this issue May 27, 2020
This replaces the partially implemented `ListTrades` call with the more
comprehensive and descriptive `TradeHistory`. This displays key info
about every trade we have made.

Closes #1232.
sangaman added a commit that referenced this issue May 29, 2020
This replaces the partially implemented `ListTrades` call with the more
comprehensive and descriptive `TradeHistory`. This displays key info
about every trade we have made.

Closes #1232.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request has PR issues with an open PR P2 mid priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants