Skip to content

Fix(ExitToNear): register the receiver account if it does not exist#151

Merged
artob merged 2 commits intodevelopfrom
fix-eth-exit-to-near
Jun 18, 2021
Merged

Fix(ExitToNear): register the receiver account if it does not exist#151
artob merged 2 commits intodevelopfrom
fix-eth-exit-to-near

Conversation

@birchmd
Copy link
Copy Markdown
Member

@birchmd birchmd commented Jun 17, 2021

I cannot take my ETH from my Aurora address and exit to my NEAR account. I get the error Smart contract panicked: ERR_ACCOUNT_NOT_EXIST. This is because my account has never held ETH before, but I think I should still be able to send it there.

See https://explorer.testnet.near.org/transactions/91qBTPUe7iA7qzDzbivJgmkv3DEYJKdVTqL9JsRRyeH6 for full transaction log.

This PR removes the panic, and thus should fix the issue. I wanted to write a test to prove that (a) this was the problem and (b) this fixes it, but writing such a test turned out to be much much harder than the solution (this is a problem itself; tests should be easy to write because then we will actually write them!). So I decided to submit this fix first and the test later because I think the fix is very high priority.

@birchmd birchmd added C-bug Category: Something isn't working. P-critical Priority: critical labels Jun 17, 2021
@birchmd birchmd requested a review from artob as a code owner June 17, 2021 20:34
Copy link
Copy Markdown
Contributor

@sept-en sept-en left a comment

Choose a reason for hiding this comment

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

Really good catch!

Thanks for checking this!

As we want to somehow ask for a storage deposit for storing the information about the account, let's register the account explicitly during the ft_transfer process. So the FungibleToken's transfer function will look something like this:

    pub fn ft_transfer(&mut self, receiver_id: &str, amount: Balance, memo: &Option<String>) {
        sdk::assert_one_yocto();
        let predecessor_account_id = sdk::predecessor_account_id();
        let sender_id = str_from_slice(&predecessor_account_id);

        if self.accounts_get(&receiver_id).is_none() {
            self.accounts_insert(&receiver_id, 0);
        }
        self.internal_transfer_eth_on_near(sender_id, receiver_id, amount, memo);
    }

We can put this functionality into EthConnector's ft_transfer instead. It's up to you. But probably putting this into FungibleToken will make the testing easier.

@artob artob assigned artob and birchmd and unassigned artob Jun 17, 2021
@birchmd birchmd changed the title Fix(ExitToNear): return 0 balance when account is not known Fix(ExitToNear): register the receiver account if it does not exist Jun 17, 2021
@birchmd
Copy link
Copy Markdown
Member Author

birchmd commented Jun 17, 2021

let's register the account explicitly during the ft_transfer process

Done in be4c7bf

@birchmd birchmd assigned artob and unassigned birchmd Jun 17, 2021
@sept-en
Copy link
Copy Markdown
Contributor

sept-en commented Jun 17, 2021

Also, I noticed that the check if the account contains a key is a little bit redundant, as the same thing is being performed in accounts_insert() method. But it makes sense to keep it as most likely we will add some logic for storage depositing there.

@artob artob merged commit 3054a27 into develop Jun 18, 2021
@artob artob deleted the fix-eth-exit-to-near branch June 18, 2021 08:13
artob added a commit that referenced this pull request Jun 18, 2021
* Implement EIP-2565 support. (#138)
* Fix CI debug pipeline. (#145)
* Enable `meta_call` only with feature flag. (#146)
* Add `mainnet`, `testnet`, and `betanet` feature flags. (#147)
* Prohibit static calls in exit precompiles. (#148)
* Fix `clippy::unnecessary_unwrap`. (#149)
* Fix all Clippy warnings. (#150)
* Create the `ExitToNear` receiver account if it does not exist. (#151)

Co-authored-by: Joshua J. Bouw <joshua@aurora.dev>
Co-authored-by: Kirill <kirill@aurora.dev>
Co-authored-by: Marcelo Fornet <marcelo@aurora.dev>
Co-authored-by: Michael Birch <michael@aurora.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-bug Category: Something isn't working. P-critical Priority: critical

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants