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

fix: Estate fingerprint calculation #2047

Merged
merged 5 commits into from
Nov 14, 2023
Merged

Conversation

meelrossi
Copy link
Contributor

We are currently having a cache problem causing the information coming from the atlas-server to be outdated. For this, in several parts of the ui (atlas, estate detaill, etc) we are showing information that is not up to date misleading the user to what they are actually seeing/buying.

To solve some of the consequences of this, this PR adds:

  • Fingerprint calculation for estates in the ui and compare it with the one generated with the contract. If this fingerprints are not the same then the information is not up to date and we prevent the user from buying the land/estate.

  • Add warning messages in the atlas and in the buy and bid page to make the users aware of the missmatch

image image

@meelrossi meelrossi changed the title Fix/fingerprint calculation fix: Estate fingerprint calculation Nov 8, 2023
@coveralls
Copy link

coveralls commented Nov 8, 2023

Coverage Status

coverage: 40.869% (+0.1%) from 40.744%
when pulling e608708 on fix/fingerprint-calculation
into 4c772cb on master.

@@ -17,6 +17,7 @@ import { VendorName } from '../../modules/vendor'
import { NFT } from '../../modules/nft/types'
import Popup from './Popup'
import './Atlas.css'
import ErrorBanner from '../ErrorBanner'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move up

@@ -28,6 +28,7 @@ import { ConfirmInputValueModal } from '../../ConfirmInputValueModal'
import { Mana } from '../../Mana'
import { Props } from './BidModal.types'
import './BidModal.css'
import ErrorBanner from '../../ErrorBanner'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move up

@@ -33,6 +33,7 @@ import { PartiallySupportedNetworkCard } from '../PartiallySupportedNetworkCard'
import { NotEnoughMana } from '../NotEnoughMana'
import { PriceHasChanged } from '../PriceHasChanged'
import { Props } from './BuyNFTModal.types'
import ErrorBanner from '../../ErrorBanner'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move up


for (const parcel of parcels) {
estateTokenIds.push(
(await contract.encodeTokenId(parcel.x, parcel.y)).toString()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this toString necessary?
The land tokenId is then used in ethers.utils.solidityKeccak256 so I believe that big number should be handled without the need of stringifying.

@fzavalia
Copy link
Contributor

fzavalia commented Nov 8, 2023

Should the message cover the 100% of the width?

Screenshot 2023-11-08 at 11 30 54

@meelrossi meelrossi merged commit 6703ad1 into master Nov 14, 2023
6 checks passed
@meelrossi meelrossi deleted the fix/fingerprint-calculation branch November 14, 2023 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants