add pairs #495

stubbrn · 2023-07-02T23:01:29Z

Ability to select Pairs

Just a mockup for now, as the logic of the future "getPairs" RPC endpoint requires to know whether there's a "hard" whitelist or not (as discussed in #491 and #178).

codecov · 2023-07-02T23:20:54Z

Codecov Report

Patch coverage: 18.97% and project coverage change: -0.61% ⚠️

Comparison is base (f7c6a1d) 61.02% compared to head (8f93cb6) 60.42%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #495      +/-   ##
==========================================
- Coverage   61.02%   60.42%   -0.61%     
==========================================
  Files         129      131       +2     
  Lines       12817    12952     +135     
==========================================
+ Hits         7822     7826       +4     
- Misses       4176     4303     +127     
- Partials      819      823       +4

Files Changed	Coverage Δ
common/types/pairs.go	`0.00% <0.00%> (ø)`
rpcclient/pairs.go	`0.00% <0.00%> (ø)`
rpc/net.go	`47.89% <5.45%> (-17.55%)`	⬇️
cmd/swapcli/main.go	`48.84% <35.00%> (-0.52%)`	⬇️
rpc/server.go	`77.20% <100.00%> (+1.42%)`	⬆️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dimalinux · 2023-07-10T22:27:29Z

Since this PR is not linked to a well documented issue, can you provide more documentation in the PR description.

Describe:

The use case that the PR is trying to solve
What a pair is
It looks like there is some concept of bidirectional liquidity in the pairs. Can you describe what those liquidity values are, probably as part of the use case, and why the values should be trusted. (Or why the values are useful even if they can't be trusted.)
What follow-up work is needed to make this PR useful. It sounds like it depends on a whitelist. Describe how you think we should solve that problem.

There are lots of images that are imported. Can you mention where they came from and what their copyright is?

stubbrn · 2023-07-11T06:41:48Z

The use case that the PR is trying to solve

Providing a RPC endpoint, a swapcli command, and a UI to see Pairs.
It wouldn't be a good UX on the UI to mashup all offers from all assets into the same table (If you have a decent amount of offers);
This is a first version to segment offers from all peers by asset.

What a pair is

A pair is simply an abstraction over offers (a summary), it doesn't contains any offers, just metadata about them, grouped by assets.
For now, it isn't stored/cached anywhere, just generated on the fly when the Pair RPC endpoint is called.

Example output of swap-cli pairs:

Pair 1:
  Name: ETH
  Token: 0x0000000000000000000000000000000000000000
  Verified: Yes
  Offers: 2
  Liquidity XMR: 220
  Liquidity ETH: 5.50

Name: Name of the asset
Token: Address of the asset
Verified: Whether the token is in the verified-list or not (Not implemented yet, that will be for another PR, always set to false, unless its ETH that's always set to true)
Offers: The amount of offers with that token
Liquidity XMR: The sum of Max Amounts of offers (of that particular asset) [How much you can get at most from all offers from a particular asset in XMR]
Liquidity ETH: Same thing but converted in ETH/Token [How much you can get at most from all offers from a particular asset in ETH/Token]

What follow-up work is needed to make this PR useful.

None

It sounds like it depends on a whitelist. Describe how you think we should solve that problem.

It doesn't depend on it, it's just that for now it's always set to false unless it's ETH.

There should be a verified-list somewhere, that is parsed by swapd.
The goal is to warn the user to double-check addresses.
Ability to change this list from the UI directly could be considered from the very first implementation.

Asset Images

https://github.com/spothq/cryptocurrency-icons/blob/master/LICENSE.md

dimalinux · 2023-07-11T19:55:31Z

Let's say someone makes the following absurd offer with a 2 million USDT to XMR exchange rate and half of a monero being offered up.

$ ./bin/swapcli make --token 0xdAC17F958D2ee523a2206206994597C13D831ec7 \
    --min 0.12 --max 0.5 --exchange-rate 2000000
Published:
  Offer ID:  0x20739776aae60ad00d7591165ef8efbbbec34b07d137f3a48169a1a234621194
  Peer ID:   12D3KooWQQRJuKTZ35eiHGNPGDpQqjpJSdaxEMJRxi6NWFrrvQVi
  Taker Min: 240000 "USDT"
  Taker Max: 1000000 "USDT"

Would this PR then report that there is a million dollars of USDT liquidity in the network?

stubbrn · 2023-07-11T20:38:33Z

Would this PR then report that there is a million dollars of USDT liquidity in the network?

Yes, and if you put something absurd for XMR you get something similar as there's no proof of funds for creating an offer.

If you don't like it we could:

Remove it
Change the definition
Find a better way but if we can't know for sure that a maker really has the funds it's probably impossible

dimalinux · 2023-07-11T22:48:59Z

Let's see what @noot or someone else thinks. Are there any exchanges that print numbers like this or that have similar concept of what "liquidity" is? At least if end users are using our software, they can't offer up more XMR than they have. Offering up exchange rates that are extremely far from what anyone would take at the moment is 100% guaranteed to happen. I've never seen it not happen on any platform. I don't think having a single offer that is legit but absurd affect a network wide statistic is desirable behavior.

noot · 2023-07-18T20:55:11Z

@stubbrn sorry for the late response. overall, I like the idea and I think it helps give a nice snapshot view of the network. but yeah, false liquidity reporting would be an issue. we could change the definition for now to say "reported liquidity" or something like that? I've thought a bit about how to add proof of XMR liquidity (#75) but there are still issues, as the maker can just transfer their funds to another account or something after sending the proof. I think for now, potentially changing the definition/adding a specific definition would be good.

stubbrn · 2023-07-19T05:02:10Z

Ok, so I'm probably going to:

Remove LiquidityETH
Rename LiquidityXMR to ReportedLiquidity (or maybe ReportedLiquidityXMR?)

If we're able to solve the problems with XMR proof-of-reserves, they could also maybe serve as part of heuristics for attempting to prevent false offers spam (you won't be able to just buy/borrow IP addresses for $10 or maybe less with tor exit nodes + generate PeerIds and cancel everything after step 1 of the protocol, as a taker could potentially see that N different makers have the same proof, so it would be more difficult to spam, although if you need to take the offer to be able to see it it wouldn't work), but that's outside of the scope of this discussion

noot

this looks good, however I just noticed the hundreds of token icons added lol. can you change this to have just the tokens currently used + common ERC20 tokens?

noot · 2023-08-16T02:09:11Z

rpc/net.go

+			log.Debugf("Failed to query peer ID %s", p)
+			continue
+		}
+		if len(msg.Offers) > 0 {


nit for readability, but can you change this to:

if len(msg.Offers) == 0 { continue } // the code that's currently in this block here

noot · 2023-08-16T02:14:53Z

rpc/net.go

+				index, exists := addrs[address]
+				pair := types.NewPair(o.EthAsset)
+				if !exists {
+					addrs[address] = index


if the value does not exist in the map, isn't index always 0? this seems off, can you simplify this logic to just have one map pairs := make(map[ethcommon.Address]*types.Pair) where the key is the ETH-asset?

noot · 2023-08-16T02:17:42Z

common/rpctypes/types.go

@@ -147,3 +147,11 @@ type AddressesResponse struct {
 type PeersResponse struct {
 	Addrs []string `json:"addresses" validate:"dive,required"`
 }
+
+// PairsRequest ...
+type PairsRequest = DiscoverRequest


since the Provides field of the request is always set to "", can you define this as a new type:

type PairsRequest struct { SearchTime uint64 `json:"searchTime"` // in seconds }

add pairs

be99723

stubbrn mentioned this pull request Jul 2, 2023

approved tokens list #491

Open

stubbrn marked this pull request as draft July 2, 2023 23:16

stubbrn added 5 commits July 7, 2023 20:25

pairs swapd + rework front

450535c

mocks

2d8e096

err AddOffer

3dfaa9d

lint

b1caca7

improve pairs, fix pairs for ERC20 tokens

bdfd3e6

stubbrn marked this pull request as ready for review July 8, 2023 22:24

Merge branch 'master' into pairs

c2a7940

removed LiquidityETH, renamed LiquidityXMR

64c92fd

noot reviewed Aug 14, 2023

View reviewed changes

remove shitcoin assets

0ed0371

noot reviewed Aug 16, 2023

View reviewed changes

stubbrn added 4 commits August 20, 2023 20:54

fixes + integration test

12b5ae7

remove printf

4d475e2

Merge branch 'master' into pairs

78b6cac

linter

8f93cb6

stubbrn requested a review from noot August 26, 2023 13:45

noot approved these changes Aug 28, 2023

View reviewed changes

noot merged commit 830c263 into AthanorLabs:master Aug 28, 2023
8 checks passed

stubbrn deleted the pairs branch August 28, 2023 02:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add pairs #495

add pairs #495

stubbrn commented Jul 2, 2023 •

edited

Loading

codecov bot commented Jul 2, 2023 •

edited

Loading

dimalinux commented Jul 10, 2023

stubbrn commented Jul 11, 2023 •

edited

Loading

dimalinux commented Jul 11, 2023

stubbrn commented Jul 11, 2023 •

edited

Loading

dimalinux commented Jul 11, 2023

noot commented Jul 18, 2023

stubbrn commented Jul 19, 2023 •

edited

Loading

noot left a comment

noot Aug 16, 2023

noot Aug 16, 2023

noot Aug 16, 2023

add pairs #495

add pairs #495

Conversation

stubbrn commented Jul 2, 2023 • edited Loading

codecov bot commented Jul 2, 2023 • edited Loading

Codecov Report

dimalinux commented Jul 10, 2023

stubbrn commented Jul 11, 2023 • edited Loading

The use case that the PR is trying to solve

What a pair is

What follow-up work is needed to make this PR useful.

It sounds like it depends on a whitelist. Describe how you think we should solve that problem.

Asset Images

dimalinux commented Jul 11, 2023

stubbrn commented Jul 11, 2023 • edited Loading

dimalinux commented Jul 11, 2023

noot commented Jul 18, 2023

stubbrn commented Jul 19, 2023 • edited Loading

noot left a comment

Choose a reason for hiding this comment

noot Aug 16, 2023

Choose a reason for hiding this comment

noot Aug 16, 2023

Choose a reason for hiding this comment

noot Aug 16, 2023

Choose a reason for hiding this comment

stubbrn commented Jul 2, 2023 •

edited

Loading

codecov bot commented Jul 2, 2023 •

edited

Loading

stubbrn commented Jul 11, 2023 •

edited

Loading

stubbrn commented Jul 11, 2023 •

edited

Loading

stubbrn commented Jul 19, 2023 •

edited

Loading