-
Notifications
You must be signed in to change notification settings - Fork 4
feat: create erc20 contract on denom metadata registration #22
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
feat: create erc20 contract on denom metadata registration #22
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Alex
Looks like nice work
I'm think I'm being dumb as I'm super tired atm, but I'm having trouble fully understanding
Most of my comments are asking for more comments and docstrings
err := e.AfterDenomMetadataCreation(sdk.Context{}, tt.args.newDenomMetadata) | ||
if tt.expectErr != "" { | ||
require.EqualError(t, err, tt.expectErr) | ||
} else { | ||
require.NoError(t, err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we replace the errors with Err* types and use require.True(t, errors.Is(err, ErrFoo))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aka no string matching plz 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed here: #23
// short-circuit: if the denom is not registered, conversion will fail | ||
// so we can continue with the rest of the stack | ||
return ack | ||
err = errorsmod.Wrapf(types.ErrTokenPairNotFound, "token pair not found for %s", coin.Denom) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
err = errorsmod.Wrapf(types.ErrTokenPairNotFound, "token pair not found for %s", coin.Denom) | |
err = errorsmod.Wrapf(types.ErrTokenPairNotFound, "token pair not found: denom: %s", coin.Denom) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed here: #23
Approved but I left an improvement for the test |
Description
Closes #dymension-rdk/311