Modify message verification inside ft_transfer_call for external receiver_id

.gitignore

@@ -23,6 +23,7 @@ etc/eth-contracts/res/
 
 # Other
 etc/state-migration-test/target/
+etc/ft-receiver/target/
 
 node_modules/*
 cache

Cargo.toml

@@ -36,5 +36,6 @@ members = [
     "engine-tests"
 ]
 exclude = [
-    "etc/state-migration-test"
+    "etc/state-migration-test",
+    "etc/ft-receiver",
 ]

engine-tests/src/test_utils/mod.rs

@@ -32,6 +32,7 @@ pub(crate) const SUBMIT: &str = "submit";
 pub(crate) mod erc20;
 pub(crate) mod exit_precompile;
 pub(crate) mod one_inch;
+pub(crate) mod rust;
 pub(crate) mod self_destruct;
 pub(crate) mod solidity;
 pub(crate) mod standard_precompiles;

engine-tests/src/test_utils/rust.rs

@@ -0,0 +1,14 @@
+use std::path::Path;
+use std::process::Command;
+
+pub fn compile<P: AsRef<Path>>(source_path: P) {
+    let output = Command::new("cargo")
+        .current_dir(source_path)
+        .args(&["build", "--target", "wasm32-unknown-unknown", "--release"])
+        .output()
+        .unwrap();
+
+    if !output.status.success() {
+        panic!("{}", String::from_utf8(output.stderr).unwrap());
+    }
+}

engine-tests/src/tests/eth_connector.rs

@@ -457,6 +457,138 @@ fn test_ft_transfer_call_eth() {
     assert_eq!(balance, transfer_amount);
 }
 
+#[test]
+fn test_ft_transfer_call_without_message() {
+    let (master_account, contract) = init(CUSTODIAN_ADDRESS);
+    let recipient_account = create_user_account(&master_account);
+    call_deposit_eth_to_near(&contract, CONTRACT_ACC);
+
+    let balance = get_eth_on_near_balance(&master_account, DEPOSITED_RECIPIENT, CONTRACT_ACC);
+    assert_eq!(balance, DEPOSITED_AMOUNT - DEPOSITED_FEE);
+
+    let balance = get_eth_on_near_balance(&master_account, CONTRACT_ACC, CONTRACT_ACC);
+    assert_eq!(balance, DEPOSITED_FEE);
+
+    let res = contract.call(
+        CONTRACT_ACC.parse().unwrap(),
+        "register_relayer",
+        &RegisterRelayerCallArgs {
+            address: validate_eth_address(CUSTODIAN_ADDRESS),
+        }
+        .try_to_vec()
+        .unwrap(),
+        DEFAULT_GAS,
+        0,
+    );
+    res.assert_success();
+
+    let transfer_amount = 50;
+    // Send to Aurora contract with wrong message should failed
+    let res = contract.call(
+        CONTRACT_ACC.parse().unwrap(),
+        "ft_transfer_call",
+        json!({
+            "receiver_id": CONTRACT_ACC,
+            "amount": transfer_amount.to_string(),
+            "msg": "",
+        })
+        .to_string()
+        .as_bytes(),
+        DEFAULT_GAS,
+        1,
+    );
+    match res.outcome().status {
+        ExecutionStatus::Failure(_) => {}
+        _ => panic!("Expected execution failure"),
+    }
+
+    // Assert balances remain unchanged
+    let balance = get_eth_on_near_balance(&master_account, DEPOSITED_RECIPIENT, CONTRACT_ACC);
+    assert_eq!(balance, DEPOSITED_AMOUNT - DEPOSITED_FEE);
+    let balance = get_eth_on_near_balance(&master_account, CONTRACT_ACC, CONTRACT_ACC);
+    assert_eq!(balance, DEPOSITED_FEE);
+
+    // Sending to random account should not change balances
+    let transfer_amount = 22;
+    let res = recipient_account.call(
+        CONTRACT_ACC.parse().unwrap(),
+        "ft_transfer_call",
+        json!({
+            "receiver_id": "some-test-acc",
+            "amount": transfer_amount.to_string(),
+            "msg": "",
+        })
+        .to_string()
+        .as_bytes(),
+        DEFAULT_GAS,
+        1,
+    );
+    res.assert_success();
+
+    // some-test-acc does not implement `ft_on_transfer` therefore the call fails and the transfer is reverted.
+    let balance = get_eth_on_near_balance(&master_account, DEPOSITED_RECIPIENT, CONTRACT_ACC);
+    assert_eq!(balance, DEPOSITED_AMOUNT - DEPOSITED_FEE);
+    let balance = get_eth_on_near_balance(&master_account, "some-test-acc", CONTRACT_ACC);
+    assert_eq!(balance, 0);
+    let balance = get_eth_on_near_balance(&master_account, CONTRACT_ACC, CONTRACT_ACC);
+    assert_eq!(balance, DEPOSITED_FEE);
+
+    // Sending to external receiver with empty message should be success
+    let dummy_ft_receiver = master_account.deploy(
+        &dummy_ft_receiver_bytes(),
+        "ft-rec".parse().unwrap(),
+        near_sdk_sim::STORAGE_AMOUNT,
+    );
+    let res = recipient_account.call(
+        CONTRACT_ACC.parse().unwrap(),
+        "ft_transfer_call",
+        json!({
+            "receiver_id": dummy_ft_receiver.account_id(),
+            "amount": transfer_amount.to_string(),
+            "msg": "",
+        })
+        .to_string()
+        .as_bytes(),
+        DEFAULT_GAS,
+        1,
+    );
+    res.assert_success();
+
+    let balance = get_eth_on_near_balance(&master_account, DEPOSITED_RECIPIENT, CONTRACT_ACC);
+    assert_eq!(balance, DEPOSITED_AMOUNT - DEPOSITED_FEE - transfer_amount);
+    let balance = get_eth_on_near_balance(
+        &master_account,
+        dummy_ft_receiver.account_id().as_ref(),
+        CONTRACT_ACC,
+    );
+    assert_eq!(balance, transfer_amount);
+    let balance = get_eth_on_near_balance(&master_account, CONTRACT_ACC, CONTRACT_ACC);
+    assert_eq!(balance, DEPOSITED_FEE);
+
+    let balance = get_eth_balance(
+        &master_account,
+        validate_eth_address(RECIPIENT_ETH_ADDRESS),
+        CONTRACT_ACC,
+    );
+    assert_eq!(balance, 0);
+
+    let balance = get_eth_balance(
+        &master_account,
+        validate_eth_address(CUSTODIAN_ADDRESS),
+        CONTRACT_ACC,
+    );
+    assert_eq!(balance, 0);
+
+    let balance = total_supply(&master_account, CONTRACT_ACC);
+    assert_eq!(balance, DEPOSITED_AMOUNT);
+
+    let balance = total_eth_supply_on_near(&master_account, CONTRACT_ACC);
+    assert_eq!(balance, DEPOSITED_AMOUNT);
+
+    let balance = total_eth_supply_on_aurora(&master_account, CONTRACT_ACC);
+    assert_eq!(balance, 0);
+}
+
 #[test]
 fn test_deposit_with_same_proof() {
     let (_master_account, contract) = init(CUSTODIAN_ADDRESS);
@@ -1269,3 +1401,11 @@ fn test_ft_transfer_wrong_u128_json_type() {
         "Expected failure as number type can't be parsed to u128",
     );
 }
+
+/// Bytes for a NEAR smart contract implementing `ft_on_transfer`
+fn dummy_ft_receiver_bytes() -> Vec<u8> {
+    let base_path = std::path::Path::new("../etc").join("ft-receiver");
+    let output_path = base_path.join("target/wasm32-unknown-unknown/release/ft_receiver.wasm");
+    crate::test_utils::rust::compile(base_path);
+    std::fs::read(output_path).unwrap()
+}

engine-tests/src/tests/state_migration.rs

@@ -1,11 +1,10 @@
 use crate::prelude::U256;
-use crate::test_utils::AuroraRunner;
+use crate::test_utils::{self, AuroraRunner};
 use aurora_engine::parameters::{InitCallArgs, NewCallArgs};
 use borsh::BorshSerialize;
 use near_sdk_sim::{ExecutionResult, UserAccount};
 use std::fs;
 use std::path::Path;
-use std::process::Command;
 
 #[test]
 fn test_state_migration() {
@@ -90,18 +89,6 @@ fn contract_bytes() -> Vec<u8> {
     let base_path = Path::new("../etc").join("state-migration-test");
     let output_path = base_path
         .join("target/wasm32-unknown-unknown/release/aurora_engine_state_migration_test.wasm");
-    compile(base_path);
+    test_utils::rust::compile(base_path);
     fs::read(output_path).unwrap()
 }
-
-fn compile<P: AsRef<Path>>(source_path: P) {
-    let output = Command::new("cargo")
-        .current_dir(source_path)
-        .args(&["build", "--target", "wasm32-unknown-unknown", "--release"])
-        .output()
-        .unwrap();
-
-    if !output.status.success() {
-        panic!("{}", String::from_utf8(output.stderr).unwrap());
-    }
-}

engine/src/connector.rs

@@ -492,26 +492,30 @@ impl EthConnectorContract {
             args.receiver_id, args.amount,
         ));
         // Verify message data before `ft_on_transfer` call to avoid verification panics
-        let message_data = self.parse_on_transfer_message(&args.msg);
-        // Check is transfer amount > fee
-        assert!(
-            args.amount > message_data.fee.as_u128(),
-            "{}",
-            ERR_NOT_ENOUGH_BALANCE_FOR_FEE,
-        );
+        if args.receiver_id.as_bytes() == &sdk::current_account_id()[..] {
+            let message_data = self.parse_on_transfer_message(&args.msg);
+            // Check is transfer amount > fee
+            assert!(
+                args.amount > message_data.fee.as_u128(),
+                "{}",
+                ERR_NOT_ENOUGH_BALANCE_FOR_FEE,
+            );
 
-        // Additional check overflow before process `ft_on_transfer`
-        // But don't check overflow for relayer
-        // Note: It can't overflow because the total supply doesn't change during transfer.
-        let amount_for_check = self
-            .ft
-            .internal_unwrap_balance_of_eth_on_aurora(message_data.recipient);
-        assert!(amount_for_check.checked_add(args.amount).is_some());
-        assert!(self
-            .ft
-            .total_eth_supply_on_aurora
-            .checked_add(args.amount)
-            .is_some());
+            // Additional check overflow before process `ft_on_transfer`
+            // But don't check overflow for relayer
+            // Note: It can't overflow because the total supply doesn't change during transfer.
+            let amount_for_check = self
+                .ft
+                .internal_unwrap_balance_of_eth_on_aurora(message_data.recipient);
+            assert!(amount_for_check.checked_add(args.amount).is_some());
+
+            // It'll call only on Aurora contract
+            assert!(self
+                .ft
+                .total_eth_supply_on_aurora
+                .checked_add(args.amount)
+                .is_some());
+        }
 
         self.ft
             .ft_transfer_call(&args.receiver_id, args.amount, &args.memo, args.msg);

engine/src/fungible_token.rs

@@ -1,7 +1,7 @@
 use crate::connector::NO_DEPOSIT;
 use crate::engine::Engine;
 use crate::json::{parse_json, JsonValue};
-use crate::parameters::{FtResolveTransfer, NEP141FtOnTransferArgs, StorageBalance};
+use crate::parameters::{NEP141FtOnTransferArgs, ResolveTransferCallArgs, StorageBalance};
 use crate::prelude::{
     sdk, storage, str_from_slice, AccountId, Address, BTreeMap, Balance, BorshDeserialize,
     BorshSerialize, EthAddress, Gas, PromiseResult, StorageBalanceBounds, StorageUsage, String,
@@ -232,16 +232,15 @@ impl FungibleToken {
         let data1: String = NEP141FtOnTransferArgs {
             amount,
             msg,
-            sender_id: receiver_id.to_string(),
+            sender_id: sender_id.to_string(),
         }
         .try_into()
         .unwrap();
 
-        let account_id = String::from_utf8(sdk::current_account_id()).unwrap();
-        let data2 = FtResolveTransfer {
+        let data2 = ResolveTransferCallArgs {
             receiver_id: receiver_id.to_string(),
             amount,
-            current_account_id: account_id,
+            sender_id: sender_id.to_string(),
         }
         .try_to_vec()
         .unwrap();

engine/src/parameters.rs

@@ -258,14 +258,6 @@ pub struct WithdrawResult {
     pub eth_custodian_address: RawAddress,
 }
 
-/// ft_resolve_transfer eth-connector call args
-#[derive(BorshSerialize)]
-pub struct FtResolveTransfer {
-    pub receiver_id: AccountId,
-    pub amount: Balance,
-    pub current_account_id: AccountId,
-}
-
 /// Fungible token storage balance
 #[derive(Default)]
 pub struct StorageBalance {
@@ -285,7 +277,7 @@ impl StorageBalance {
     }
 }
 
-/// resolve_transfer eth-connector call args
+/// ft_resolve_transfer eth-connector call args
 #[derive(BorshSerialize, BorshDeserialize)]
 pub struct ResolveTransferCallArgs {
     pub sender_id: AccountId,