DeXter-on-Radix / website

Community built order book dex on Radix.
https://dexteronradix.com/
Other
11 stars 21 forks source link

Mauro cannot cancel transactions #520

Open dcts opened 1 month ago

dcts commented 1 month ago

This call currently does not return any orders, making it impossible for Mauro to cancel transaction. But the account clearly has 12 adex order receipts inside of it: https://dashboard.radixdlt.com/account/account_rdx168wh0jqpgdyzajnqznzvje49edtx2gg9wdk49wptxar8lu9llz4sjk/nfts

const account2 = {
    address:
      "account_rdx168wh0jqpgdyzajnqznzvje49edtx2gg9wdk49wptxar8lu9llz4sjk",
    label: "mj",
    appearanceId: 0,
  };
  const pairAddress2 =
    "component_rdx1czgjmu4ed5hp0vn97ngsf6mevq9tl0v8rrh2yq0f4dnpndggk7j9pu";
  const apiResponse = await adex.getAccountOrders(
    account2.address,
    pairAddress2,
    0
  );

ToDo:

fliebenberg commented 1 month ago

I just tried to get an extract of orders for this account on postman and it works. Is this still an issue for Mauro?

fliebenberg commented 1 month ago

You can always test API calls on postman here: https://www.postman.com/alphadex/workspace/public-workspace/overview

fliebenberg commented 1 month ago

OK, figured out what the problem might be. The api call in the code above includes "0" as the startFrom parameter. That causes the api call to fail ("Could not find order with specified startFrom id 0"). You should just leave the startFrom parameter out if you do not want to start from a specific order. If a startFrom order is specified, the API tries to find that order in the results and gives an error if ot cannot find it. startFrom is used when the API returns a startFrom in the result, meaning it found too many results and you need to call it again with the startFrom parameter to get more results.

dcts commented 1 month ago

Yes this is still an issue.

The api call in the code above includes "0" as the startFrom parameter. That causes the api call to fail

@fliebenberg this is really weird, because that is the code we use for the dexter app and it works for everyone. See here: https://github.com/DeXter-on-Radix/website/blob/512badd0382548c5fd0b5958acc06dfe840f63be/src/app/state/accountHistorySlice.ts#L63

Is it possible that it works in some circumstances but fails in others?

Do you suggest to skip the startFrom input in the first call and add logic to keep fetching orders if the result contains a startFrom parameter?

fliebenberg commented 1 month ago

It seems you just discovered a bug in the alphadex-sdk code. The sdk is sending a "from" field to the Alphadex API, which is ignoring the "from" data field as it is expecting a "startFrom" field. I will need to update the SDK to specify the field correctly, but also to ignore a value of 0 (just to ensure backward compatibility with apps that had 0). Since we never use the field (it is always 0), I suggest we omit it in our code (just send 2 params to the function).

fliebenberg commented 1 month ago

As for the Mauro's problem, we need to check if it is still happening. It might have been just a once off when the backend server was unavailable. If I check now it looks like it should work fine.

fliebenberg commented 1 month ago

Have updated the alphadex-sdk (new version 0.12.15), which will actually use the from field in future. When it is 0, like in the Dexter code, it will be ignored and all orders will be fetched.

dcts commented 1 month ago

Ok let me update our repo and we can see whether it fixes the issue for Mauro.

fliebenberg commented 1 month ago

Could you please check with him if the issue is still there before you update. I am pretty sure the update wont change anything, but to be sure, lets check if the issue is still there before the update.

dcts commented 1 month ago

We tested it multiple times so its unlikely the server was down during all tests, but will double check to be sure!

In case the issue still exists: do you want us to test the PR locally on mainnet (which is more time consuming) or should we merge the PR #559 and let mauro test on dexteronradix.com?

The question I guess is: do we still want to update adex to 0.12.15 even if it doesnt fix Mauros issue?

fliebenberg commented 1 month ago

I would say we can update just to be on the most updated version and it is very easy to do. I am much more interested in what is going wrong for Mauro though as it would be very funny if only one person was experiencing the issue.

dcts commented 1 month ago

@fliebenberg reopened as Mauro still faces this issue (even after the adex update). I plan to investigate this further this week, but if you have any suggeetions/ideas please drop them in the chat. It's a really weird bug.