neon-wallet icon indicating copy to clipboard operation
neon-wallet copied to clipboard

Incorrect Transaction Totals Displayed on Activity Display for N3 Wallet

Open jseagrave21 opened this issue 1 year ago • 15 comments

Wallet version or commit SHA: 2.23.10

Environment: Linux Mint 21.2 Cinnamon

Prerequisites: Must have a N3 wallet with multiple transactions.

~I am not sure how many. In my example, it seems that the total displayed is capped at 188.~

Update: You need to have non-NEP-17 transfers.

Reproduction steps:

  1. Open the N3 Wallet
  2. Navigate to the Activity tab
  3. Load more transactions

In my example N3 wallet, only 188 of the total 230 transactions are displayed as the total number of available transactions. Then when the total transactions are loaded, the number of available transactions goes to 0 and the option to load more transactions remains displayed.

This seems to be a N3 problem only. The N2 wallet shows all transactions and when all are loaded, the displayed total matches the total number of available transactions and the option to load more is no longer available.

Screenshots/Video:

  • N3 wallet with more transactions displayed than available not all txs displayed

  • N3 wallet with all transactions displayed all txs loaded

  • N2 wallet with some transactions loaded Partial Load of Legacy

  • N2 wallet with all transactions displayed Legacy

jseagrave21 avatar Jan 06 '24 20:01 jseagrave21

While trying to research a solution to this problem, I discovered that neon-wallet is using a deprecated method from the rest API. Specifically, we are using addressTXFull, which is annotated as being deprecated here. However, if you trying to use addressTransactions as directed, the code breaks because none of the metadata is available in the new method. Additionally, the totalCount displayed for the same address referenced above using addressTransactions is 181 which is still wrong. @lllwvlvwlll @ixje

jseagrave21 avatar Jan 06 '24 22:01 jseagrave21

the code breaks because none of the metadata is available in the new method You mean the transaction history page?

The information in addressTransactions differs slightly in the sense that invocations[x].metadata doesn't exist because []invocations is always empty. The data in that field is mostly the same as what is stored in the []transfers field, with at times some more human friendly text. I ended up not porting the invocations field from the /v1 API to the /v2 API and suggested to consume it from transfers.

~~As for the totalCount, that value for the /v1 API seems to be wrong when I tested it (for a given address it was returning 5 pages of 15 items while reporting that the total count is 60).~~ We can ignore this, there is some pagination "issue" but that doesn't affect anyone using the SDK

The addressTransactions endpoint uses the /v2 API. It returns 181 (?) for which you say it is still wrong. What should be the right number? Can you share a public address with this same issue so it can be investigated?

ixje avatar Jan 08 '24 14:01 ixje

You mean the transaction history page?

Yes.

The data in that field is mostly the same as what is stored in the transfers field

I looked in the transfers field for the similar data and couldn't not find all the data referenced in neon-wallet now. Specifically, I was looking for the name of the invocation which we use often to describe the transaction. For example, above you see the screen shot of "CompoundingNeo | compound", where compound is the type of invocation.

Can you share a public address with this same issue so it can be investigated?

The address I have been investigating is NdFBzZVMUXjdwnaDfeMK4zEVYEzjiVhFBt

In the example, I took screenshots where the total was 230 transactions.

jseagrave21 avatar Jan 12 '24 01:01 jseagrave21

Specifically, I was looking for the name of the invocation which we use often to describe the transaction. For example, above you see the screen shot of "CompoundingNeo | compound", where compound is the type of invocation.

@melanke can you help me understand where that comes from? I've never heard of a "compounding" transaction so no Idea what this is supposed to be/do

ixje avatar Jan 12 '24 08:01 ixje

@jseagrave21 So I've been looking at the address provided and I've identified some differences in the data returned between the /v1 and /v2 API endpoints. Some are known (e.g. /v2 currently does not index NEP-11 data, so those activities are missing in the history) others are a little surprising and I've started a discussion internally.

As for the CompoundingNeo | compound I found that CompoundingNeo is the name of the smart contract called and compound is the notification event name. The "compound" event exists in the list of notifications (note: with the contract hash, not the contract name), but /v1 has data in the invocations list that also has the contract name and for /v2 (as mentioned previously) that list is always empty.

ixje avatar Jan 15 '24 10:01 ixje

@ixje

others are a little surprising

That's great! I'm glad this discussion is turning out to be useful on a bigger scale than I was thinking.

The "compound" event exists in the list of notifications (note: with the contract hash, not the contract name)

So, if we were using addressTransactions it looks like we would be able to get the same information to display on the Activity page as we have currently, with some extra lookups to get the contract name from the hash. However, my concern is the repeatability. Could we do that for every transaction?

Right now, my understanding is that the neon-wallet uses addressTXFull in tandem with computeN3Activity to create the entries for the Activity page. I think that if we were to switch to using the addressTransactions function, we may not be able to present the same information because the number of notifications from an invocation is variable so trying to parse the defining invocation type (e.g. compound) from the list of notifications would not be repeatable for every type of transaction since this would be a moving target. At the very least, we cannot switch to using addressTransactions with the neon-wallet code in its current state.

jseagrave21 avatar Jan 15 '24 11:01 jseagrave21

@ixje

For this specific issue (i.e. totalCount discrepancies in general and between /v1 and /v2), I think I could fix the totalCount using addressTransactions to generate it and continue using addressTXFull to parse the entries for the activity page. My question is if addressTransactions is giving the correct totalCount.

jseagrave21 avatar Jan 15 '24 11:01 jseagrave21

address transactions does not exist on the v1 API. On the v2 API it is really just an alias for tx_addressfull (which was poorly named imo). Therefore, it is not going to give the correct totalCount because it does not include NEP-11 events.

ixje avatar Jan 15 '24 11:01 ixje

Therefore, it is not going to give the correct totalCount because it does not include NEP-11 events.

Okay, that's good to know. Unfortunately, I am not sure how to proceed based on that. I think the question becomes should we display the accurate totalCount (including NEP-11 events) for the total number of transactions for an address or should we use a different method and only display the transactions available on Dora? (This assumes that the totalCount from addressTransactions accurately describes the total number of transactions viewable on Dora for an address.)

jseagrave21 avatar Jan 15 '24 12:01 jseagrave21

My recommended solution is to remove the transaction count and total count from the Activity Page. This is similar to what Dora displays now when viewing transactions for a specific wallet (i.e just a "load more" option at the bottom of the page).

By removing the total count, we can still display all the transactions for both Neo Legacy and Neo3 on neon-wallet despite Dora not displaying NEP-11 transactions on the Transactions page(s).

jseagrave21 avatar Jan 28 '24 04:01 jseagrave21

This issue is stale because it has been open for 14 days with no activity. It will be automatically closed in 7 days from now if no activity is detected.

github-actions[bot] avatar Mar 30 '24 00:03 github-actions[bot]

Up

jseagrave21 avatar Mar 30 '24 03:03 jseagrave21

This issue is stale because it has been open for 14 days with no activity. It will be automatically closed in 7 days from now if no activity is detected.

github-actions[bot] avatar May 31 '24 00:05 github-actions[bot]

Up

jseagrave21 avatar May 31 '24 01:05 jseagrave21

This issue is stale because it has been open for 14 days with no activity. It will be automatically closed in 7 days from now if no activity is detected.

github-actions[bot] avatar Aug 01 '24 00:08 github-actions[bot]

This issue was closed because it has been inactive for 7 days since being marked as stale.

github-actions[bot] avatar Aug 16 '24 00:08 github-actions[bot]