ripple / explorer

Open Source XRP Ledger Explorer
https://livenet.xrpl.org/
MIT License
100 stars 63 forks source link

Parsing NFTokenMint transaction is not correct. #411

Closed mahiros-tech closed 1 year ago

mahiros-tech commented 1 year ago

To make things simple; Just compare these 2 links; https://nft-devnet.xrpl.org/transactions/4E0EB5F23D248740CB8FC28D1003CEFE841E21811FE2EA4B195CFE1B0BC54219 https://xls20.bithomp.com/explorer/4E0EB5F23D248740CB8FC28D1003CEFE841E21811FE2EA4B195CFE1B0BC54219

As you can see here, bithomp returns NFTokenID as [000D0000B9BD7D214128A91ECECE5FCFF9BDB0D043567C51CFBEC443000063A7] and nft-devnet.xrpl.org returns NFTokenID as [000D0000B9BD7D214128A91ECECE5FCFF9BDB0D043567C51CEE49E170000297B]

After short time research bithomp returns the correct NFtokenID. And I can see txSummarize module of this ripple explorer is not correct.(NFTokenMint module).

Hope to be fixed soon. Thanks.

ckniffen commented 1 year ago

We just created a test case for this. https://github.com/ripple/explorer/pull/325. I wonder if your transaction is hitting this edge case fixed here. This was deployed yesterday to production.

Tomorrow i will check this against the code before that was merged to see if it hits this edge case.

Can you elaborate on how the code is incorrect?

ckniffen commented 1 year ago

@mahiros-tech nvm I think I see the issue. Thank you for submitting this ticket.

mahiros-tech commented 1 year ago

Um, it's not fixed on my side, and had no time to check it deeply. But I am checking it here. and it's fixed and already deployed to the github?

ckniffen commented 1 year ago

It was deployed yesterday. What I am thinking is we aren't taking into account the situation where two NFTokenPages are modified and one is created.

ckniffen commented 1 year ago

@mahiros-tech It happens when an NFT is added to a new page in between two existing pages. We have a fix coming.

ckniffen commented 1 year ago

@mahiros-tech this has been fixed and deployed