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

WIP: Set proper account types + pubkeys #51

Closed
wants to merge 4 commits into from

Conversation

ChristianBorst
Copy link
Collaborator

@ChristianBorst ChristianBorst commented Feb 27, 2024

2 problems solved with this PR:

  1. Cosmos pubkeys can be stored under EthAccounts. This isn't a problem for the chain but is a problem for Cosmos wallets getting the wrong account type when querying the account number + sequence - they error out despite the value being in the returned type
  2. EVM Txs do not assign a pubkey, but they increment the account's sequence. This is not a problem for the chain but it is confusing and seems like bad behavior

The initial MsgLiquify implementation had the NFT deployed in the EVM by
the microtx module, and ignored the gas cost incurred by the EVM
operations. A contract deploy is a pretty hefty op to allow for free so
to prevent spam we need to charge the user.
MsgLiquify deploys a NFT contract with address derived from the user's
EVM address and their nonce. However the nonce is only incremented on
every Tx, so if a user submits multiple MsgLiquify in the same Tx there
may be deployment issues and undefined behavior.
Keplr (and likely other wallets) expect to see Cosmos account types
(e.g. vesting or BaseAccount types) for Cosmos chains but error when the
sequence query returns an EthAccount type. To avoid this issue we
default to Cosmos BaseAccounts on creation but update the account type
when setting the PubKey if it is an Ethermint account. Vesting accounts
are left unchanged.
@ChristianBorst ChristianBorst deleted the christianborst/account-types branch March 6, 2024 17:35
@ChristianBorst
Copy link
Collaborator Author

Closed in favor of a branch with a better implementation.

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.

1 participant