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

near-contract-standards/non-fungible-token: NEAR values instead of yoctoNEAR are returned in a storage management error message #1160

Closed
yandrushchak opened this issue Mar 21, 2024 · 8 comments
Assignees

Comments

@yandrushchak
Copy link

Package version: near-contract-standards = "5.0.0"

Steps to reproduce:

  1. Implement standard NFT contract from near-contract-standards, e.g. example from this repo - https://github.com/near/near-sdk-rs/tree/near-sdk-v5.0.0/examples/non-fungible-token.
  2. Call method that requires attached storage with not enough deposit, e.g. nft_approve.

Actual result: Smart contract panicked: Must attach <0.001 NEAR yoctoNEAR to cover storage
Expected result: Message with proper amount of yoctoNEAR is returned.

Additional info:

This happens because panic message uses NearToken, which is formatted as NEAR amount - https://github.com/near/near-sdk-rs/blob/near-sdk-v5.0.0/near-contract-standards/src/non_fungible_token/utils.rs#L36.

@frol
Copy link
Collaborator

frol commented Mar 28, 2024

@yandrushchak Good catch!

Let's extract the exact-display implementation from near-cli-rs into NearToken::exact_amount_display(&self) method in https://github.com/near/near-token-rs and use it here as:

format!("Must attach {} to cover storage", required_cost.exact_amount_display())

Would you have time to contribute the fix? I am happy to help you

@TropicalDog17
Copy link
Contributor

TropicalDog17 commented Jul 29, 2024

\cc @frol
I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I'm a software engineer who knows Rust well and has helped with many Rust open source projects(ibc-rs, reth, foundry-rs).

How I plan on tackling this issue

First I will try to reproduce the issue, and if this issue still exists I will follow the instruction and fix the issue. If not it can be closed.

@nicosanc
Copy link

@yandrushchak Good catch!

Let's extract the exact-display implementation from near-cli-rs into NearToken::exact_amount_display(&self) method in https://github.com/near/near-token-rs and use it here as:

format!("Must attach {} yoctoNEAR to cover storage", required_cost.exact_amount_display())

Would you have time to contribute the fix? I am happy to help you

Is the method "NearToken::exact_amount_display(&self)" the same as the Display implementation shown in src/trait_impls/display.rs? I'm having trouble localizing that specific method in near-token-rs

@frol
Copy link
Collaborator

frol commented Jul 29, 2024

@nicosanc The point is to copy/move the implementation from https://github.com/near/near-cli-rs/blob/7fca81bb5544b40743290d3a951c1f1ff31a95d9/src/types/near_token.rs#L18-L35 to https://github.com/near/near-token-rs, and then use it here:

format!("Must attach {} yoctoNEAR to cover storage", required_cost)

@lauchaves
Copy link
Contributor

lauchaves commented Jul 29, 2024

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hey! I'm Lau Chaves, and I would like to contribute to this issue! I’m a member of Dojo Coding Costa Rica!

I have over 5 years of experience working with JavaScript, React, TypeScript, ruby... I’m excited to dive deeper into smart contracts, and this looks like a fantastic chance to learn a bit more!

How I plan on tackling this issue

Based on the intrucstions mentioned on the comments above and the issue itself, Here’s how I would approach the problem:

  • Extract Code: Obtain the exact_amount_display method from near-cli-rsand add it to the NearToken struct in near-token-rs.
  • Update Panic Message: Modify the panic message in the non_fungible_token contract to use:
    format!("Must attach {} to cover storage", required_cost.exact_amount_display())
  • Test: Ensure that the updated message now correctly shows the required amount in yoctoNEAR.

@frol
Copy link
Collaborator

frol commented Jul 30, 2024

@TropicalDog17 I think this is too simple for Rust devs, and given that you already have another issue assigned to, I decided to give @lauchaves a chance here.

@lauchaves Please, go ahead!

@lauchaves
Copy link
Contributor

@frol awesome, thank you! I'll get started on it right away.

@lauchaves
Copy link
Contributor

Hey @frol would you mind taking a look at this near-token pull request ?

@frol frol closed this as completed Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants