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

refactor: PSP22Error conversion & assertion #324

Merged

Conversation

chungquantin
Copy link
Collaborator

@chungquantin chungquantin commented Oct 3, 2024

Refactoring the pop-api error assertions to make it readable instead of comparing with raw bytes. This PR relies on the PR in pop-drink: r0gue-io/pop-drink#14

About the implementation

Screenshot 2024-10-08 at 00 43 37

Currently, in the example PR, the pop-api error assertion involves converting pop_api::primitives::Module to a u32. Our goal is to simplify the assertion to make it more readable, aiming for the following conversion chain:

runtime::<Pallet>::Error -> pop_api::primitives::<version>::Error -> u32

Final outcome

How to assert the PSP22Error <> pop_drink::Error?

use drink::devnet::error::{v0::{assert_runtime_err, RuntimeError},  Assets, AssetsError::AssetNotLive};

assert_runtime_err!(
	transfer(&mut session, BOB, AMOUNT / 2),
	RuntimeError::Module(Assets(AssetNotLive)),
);

If the PalletError assertion failed

---- tests::burn_fails_with_no_permission stdout ----
thread 'tests::burn_fails_with_no_permission' panicked at tests.rs:910:13:
assertion `left == right` failed
  left: Module(RuntimeError::Assets(NoPermission))
 right: Module(RuntimeError::Assets(IncorrectStatus))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

If the DispatchError assertion failed (other than PalletError)

---- tests::mint_fails_with_arithmetic_overflow stdout ----
thread 'tests::mint_fails_with_arithmetic_overflow' panicked at tests.rs:644:5:
assertion `left == right` failed
  left: Api(Arithmetic(Overflow))
 right: Api(Arithmetic(Underflow))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.60%. Comparing base (56f1076) to head (168cb0e).

@@                       Coverage Diff                        @@
##           chungquantin/feat-psp22_example     #324   +/-   ##
================================================================
  Coverage                            51.60%   51.60%           
================================================================
  Files                                   48       48           
  Lines                                 4910     4910           
  Branches                              4910     4910           
================================================================
  Hits                                  2534     2534           
  Misses                                2327     2327           
  Partials                                49       49           

@chungquantin chungquantin changed the title refactor: pop-api error refactor: PSP22Error conversion & assertion Oct 7, 2024
@chungquantin chungquantin changed the title refactor: PSP22Error conversion & assertion refactor: PSP22Error conversion & assertion Oct 7, 2024
Copy link
Collaborator

@Daanvdplas Daanvdplas left a comment

Choose a reason for hiding this comment

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

Will review the pop drink PR now quickly but looking great!

pop-api/src/v0/fungibles/errors.rs Show resolved Hide resolved
pop-api/examples/fungibles/tests.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@Daanvdplas Daanvdplas left a comment

Choose a reason for hiding this comment

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

LGTM!

@chungquantin chungquantin merged commit 93de4b8 into chungquantin/feat-psp22_example Oct 10, 2024
13 checks passed
@chungquantin chungquantin deleted the chungquantin/refactor-api_error branch October 10, 2024 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants