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

dapp-feat: ERC721 TokenPermission feature complere #411

Merged

Conversation

quiet-node
Copy link
Member

Description:

  • complete ERC721 TokenPermission feature
  • added HEDERA_COMMON_WALLET_REVERT_REASONS constant object for different revert reasons returned from wallet

Related issue(s): #314

Fixes partial #322

UI Demo

Screen.Recording.2023-09-19.at.2.40.06.PM.mov

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@quiet-node quiet-node added enhancement New feature or request P2 Tooling tooling labels Sep 19, 2023
@quiet-node quiet-node added this to the 0.5.0 milestone Sep 19, 2023
@quiet-node quiet-node requested a review from a team as a code owner September 19, 2023 19:46
@quiet-node quiet-node self-assigned this Sep 19, 2023
@quiet-node quiet-node requested a review from ebadiere September 19, 2023 19:46
@github-actions
Copy link

github-actions bot commented Sep 19, 2023

Test Results

  17 files  +  4    70 suites  +8   9m 38s ⏱️ + 2m 3s
197 tests ±  0  188 ✔️  -   1  6 💤 ±0  3 +1 
231 runs  +34  220 ✔️ +31  6 💤 ±0  5 +3 

For more details on these failures, see this check.

Results for commit 1bb4b13. ± Comparison against base commit 75647d1.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Looking good.
One suggestion

REJECT: {
// @notice 4001 error code is returned when a metamask wallet request is rejected by the user
// @notice See https://docs.metamask.io/wallet/reference/provider-api/#errors for more information on the error returned by Metamask.
message: '4001',
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be code. See https://eips.ethereum.org/EIPS/eip-1193#provider-errors

Suggested change
message: '4001',
code: '4001',

Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Unblocking.
Request for a ticket to come back to this

'TRANSFER_EXCEEDS_BALANCE',
];

let errorDescription = HEDERA_COMMON_WALLET_REVERT_REASONS.DEFAULT.description;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Open a ticket to improve the handleAPIError() to map codes to error names.
This works for now but we can simplify later without special cases

@quiet-node quiet-node merged commit c96f7eb into main Sep 19, 2023
@quiet-node quiet-node deleted the 322-Add-contract-interacting-features-to-ERC721Contract branch September 19, 2023 22:54
mshakeg pushed a commit to mshakeg/hedera-smart-contracts that referenced this pull request Oct 14, 2023
* dapp-feat: ERC721 Approve feature complete

Signed-off-by: Logan Nguyen <[email protected]>

* dapp-update: added HEDERA_COMMON_WALLET_REVERT_REASONS constant object

Signed-off-by: Logan Nguyen <[email protected]>

* dapp-update: replaced `message` with `code`

Signed-off-by: Logan Nguyen <[email protected]>

---------

Signed-off-by: Logan Nguyen <[email protected]>
Signed-off-by: Mo Shaikjee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P2 Tooling tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants