Skip to content

Commit

Permalink
EVM: Add validation pipeline for transferdomain transparent tx, fix f…
Browse files Browse the repository at this point in the history 
…ailing EVM tests (#2448)

* Add valid transparent tx pipeline and prevalidation of transferdomain

* lint: remove unused include

---------

Co-authored-by: Bushstar <[email protected]>
  • Loading branch information
@sieniven @Bushstar
sieniven and Bushstar authored Sep 14, 2023
1 parent f4edb77 commit a3145b1
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 79 deletions.
96 changes: 95 additions & 1 deletion lib/ain-evm/src/core.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use std::cmp::Ordering;
use std::{path::PathBuf, sync::Arc};

use ain_contracts::{get_transferdomain_contract, FixedContract};
use anyhow::format_err;
use ethereum::{AccessList, Account, Block, Log, PartialHeader, TransactionV2};
use ethereum::{AccessList, Account, Block, Log, PartialHeader, TransactionAction, TransactionV2};
use ethereum_types::{Bloom, BloomInput, H160, H256, U256};
use log::{debug, trace};
use vsdb_core::vsdb_set_base_dir;
Expand Down Expand Up @@ -347,6 +348,99 @@ impl EVMCoreService {
})
}

/// Validates a raw transfer domain tx.
///
/// The validation checks of the tx before we consider it to be valid are:
/// 1. Account nonce check: verify that the tx nonce must be more than or equal to the account nonce.
/// 2. tx value check: verify that amount is set to zero.
/// 3. Verify that transaction action is a call to the transferdomain contract address.
///
/// # Arguments
///
/// * `tx` - The raw tx.
/// * `queue_id` - The queue_id queue number.
///
/// # Returns
///
/// Returns the validation result.
///
/// # Safety
///
/// Result cannot be used safety unless cs_main lock is taken on C++ side
/// across all usages. Note: To be replaced with a proper lock flow later.
///
pub unsafe fn validate_raw_transferdomain_tx(&self, tx: &str, queue_id: u64) -> Result<()> {
debug!("[validate_raw_transferdomain_tx] queue_id {}", queue_id);
debug!(
"[validate_raw_transferdomain_tx] raw transaction : {:#?}",
tx
);
let signed_tx = SignedTx::try_from(tx)
.map_err(|_| format_err!("Error: decoding raw tx to TransactionV2"))?;
debug!(
"[validate_raw_transferdomain_tx] signed_tx : {:#?}",
signed_tx
);

let state_root = self.tx_queues.get_latest_state_root_in(queue_id)?;
debug!(
"[validate_raw_transferdomain_tx] state_root : {:#?}",
state_root
);

let backend = self.get_backend(state_root)?;

let signed_tx: SignedTx = tx.try_into()?;
let nonce = backend.get_nonce(&signed_tx.sender);
debug!(
"[validate_raw_transferdomain_tx] signed_tx.sender : {:#?}",
signed_tx.sender
);
debug!(
"[validate_raw_transferdomain_tx] signed_tx nonce : {:#?}",
signed_tx.nonce()
);
debug!("[validate_raw_transferdomain_tx] nonce : {:#?}", nonce);

// Validate tx nonce
if nonce > signed_tx.nonce() {
return Err(format_err!(
"Invalid nonce. Account nonce {}, signed_tx nonce {}",
nonce,
signed_tx.nonce()
)
.into());
}

// Validate tx value equal to zero
if signed_tx.value() != U256::zero() {
debug!("[validate_raw_transferdomain_tx] value not equal to zero");
return Err(format_err!("value not equal to zero").into());
}

// Verify transaction action and transferdomain contract address
let FixedContract { fixed_address, .. } = get_transferdomain_contract();
match signed_tx.action() {
TransactionAction::Call(address) => {
if address != fixed_address {
return Err(format_err!(
"Invalid call address. Fixed address: {:#?}, signed_tx call address: {:#?}",
fixed_address,
address
)
.into());
}
}
_ => {
return Err(
format_err!("tx action not a call to transferdomain contract address").into(),
)
}
}

Ok(())
}

pub fn logs_bloom(logs: Vec<Log>, bloom: &mut Bloom) {
for log in logs {
bloom.accrue(BloomInput::Raw(&log.address[..]));
Expand Down
54 changes: 53 additions & 1 deletion lib/ain-rs-exports/src/evm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ pub fn evm_unsafe_try_add_balance_in_q(
queue_id: u64,
raw_tx: &str,
native_hash: &str,
pre_validate: bool,
) {
let Ok(signed_tx) = SignedTx::try_from(raw_tx) else {
return cross_boundary_error_return(result, "Invalid raw tx");
Expand All @@ -374,12 +375,29 @@ pub fn evm_unsafe_try_add_balance_in_q(
signed_tx: Box::new(signed_tx),
direction: TransferDirection::EvmIn,
}));

unsafe {
match SERVICES
.evm
.core
.validate_raw_transferdomain_tx(raw_tx, queue_id)
{
Ok(()) => {}
Err(e) => {
debug!("validate_raw_transferdomain_tx failed with error: {e}");
return cross_boundary_error_return(result, e.to_string());
}
}

if pre_validate {
return cross_boundary_success(result);
}

match SERVICES
.evm
.push_tx_in_queue(queue_id, queue_tx, native_hash)
{
Ok(()) => cross_boundary_success_return(result, ()),
Ok(()) => cross_boundary_success(result),
Err(e) => cross_boundary_error_return(result, e.to_string()),
}
}
Expand Down Expand Up @@ -409,6 +427,7 @@ pub fn evm_unsafe_try_sub_balance_in_q(
queue_id: u64,
raw_tx: &str,
native_hash: &str,
pre_validate: bool,
) -> bool {
let Ok(signed_tx) = SignedTx::try_from(raw_tx) else {
return cross_boundary_error_return(result, "Invalid raw tx");
Expand All @@ -421,6 +440,22 @@ pub fn evm_unsafe_try_sub_balance_in_q(
}));

unsafe {
match SERVICES
.evm
.core
.validate_raw_transferdomain_tx(raw_tx, queue_id)
{
Ok(()) => {}
Err(e) => {
debug!("validate_raw_transferdomain_tx failed with error: {e}");
return cross_boundary_error_return(result, e.to_string());
}
}

if pre_validate {
return cross_boundary_success_return(result, true);
}

match SERVICES
.evm
.push_tx_in_queue(queue_id, queue_tx, native_hash)
Expand Down Expand Up @@ -930,6 +965,7 @@ pub fn evm_try_bridge_dst20(
native_hash: &str,
token_id: u64,
out: bool,
pre_validate: bool,
) {
let native_hash = XHash::from(native_hash);
let contract_address = match ain_contracts::dst20_address_from_token_id(token_id) {
Expand All @@ -946,6 +982,22 @@ pub fn evm_try_bridge_dst20(
}));

unsafe {
match SERVICES
.evm
.core
.validate_raw_transferdomain_tx(raw_tx, queue_id)
{
Ok(()) => {}
Err(e) => {
debug!("validate_raw_transferdomain_tx failed with error: {e}");
return cross_boundary_error_return(result, e.to_string());
}
}

if pre_validate {
return cross_boundary_success(result);
}

match SERVICES
.evm
.push_tx_in_queue(queue_id, system_tx, native_hash)
Expand Down
3 changes: 3 additions & 0 deletions lib/ain-rs-exports/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,14 @@ pub mod ffi {
queue_id: u64,
raw_tx: &str,
native_hash: &str,
pre_validate: bool,
);
fn evm_unsafe_try_sub_balance_in_q(
result: &mut CrossBoundaryResult,
queue_id: u64,
raw_tx: &str,
native_hash: &str,
pre_validate: bool,
) -> bool;
fn evm_unsafe_try_validate_raw_tx_in_q(
result: &mut CrossBoundaryResult,
Expand Down Expand Up @@ -230,6 +232,7 @@ pub mod ffi {
native_hash: &str,
token_id: u64,
out: bool,
pre_validate: bool,
);
fn evm_try_is_dst20_deployed_or_queued(
result: &mut CrossBoundaryResult,
Expand Down
8 changes: 4 additions & 4 deletions src/masternodes/consensus/xvm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,13 +228,13 @@ Res CXVMConsensus::operator()(const CTransferDomainMessage &obj) const {
// Add balance to ERC55 address
auto tokenId = dst.amount.nTokenId;
if (tokenId == DCT_ID{0}) {
evm_unsafe_try_add_balance_in_q(result, evmQueueId, HexStr(dst.data), tx.GetHash().GetHex());
evm_unsafe_try_add_balance_in_q(result, evmQueueId, HexStr(dst.data), tx.GetHash().GetHex(), evmPreValidate);
if (!result.ok) {
return Res::Err("Error bridging DFI: %s", result.reason);
}
}
else {
evm_try_bridge_dst20(result, evmQueueId, HexStr(dst.data), tx.GetHash().GetHex(), tokenId.v, true);
evm_try_bridge_dst20(result, evmQueueId, HexStr(dst.data), tx.GetHash().GetHex(), tokenId.v, true, evmPreValidate);
if (!result.ok) {
return Res::Err("Error bridging DST20: %s", result.reason);
}
Expand Down Expand Up @@ -270,15 +270,15 @@ Res CXVMConsensus::operator()(const CTransferDomainMessage &obj) const {
// Subtract balance from ERC55 address
auto tokenId = dst.amount.nTokenId;
if (tokenId == DCT_ID{0}) {
if (!evm_unsafe_try_sub_balance_in_q(result, evmQueueId, HexStr(src.data), tx.GetHash().GetHex())) {
if (!evm_unsafe_try_sub_balance_in_q(result, evmQueueId, HexStr(src.data), tx.GetHash().GetHex(), evmPreValidate)) {
return DeFiErrors::TransferDomainNotEnoughBalance(EncodeDestination(dest));
}
if (!result.ok) {
return Res::Err("Error bridging DFI: %s", result.reason);
}
}
else {
evm_try_bridge_dst20(result, evmQueueId, HexStr(src.data), tx.GetHash().GetHex(), tokenId.v, false);
evm_try_bridge_dst20(result, evmQueueId, HexStr(src.data), tx.GetHash().GetHex(), tokenId.v, false, evmPreValidate);
if (!result.ok) {
return Res::Err("Error bridging DST20: %s", result.reason);
}
Expand Down
29 changes: 1 addition & 28 deletions test/functional/feature_dst20.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ def test_bridge_when_no_balance(self):
assert_equal(
self.btc.functions.totalSupply().call()
/ math.pow(10, self.btc.functions.decimals().call()),
Decimal(5.5),
Decimal(3.5),
)
[afterAmount] = [x for x in self.node.getaccount(self.address) if "BTC" in x]
assert_equal(beforeAmount, afterAmount)
Expand Down Expand Up @@ -597,32 +597,6 @@ def test_invalid_token(self):
],
)

def test_transfer_to_token_address(self):
self.nodes[0].transferdomain(
[
{
"src": {"address": self.address, "amount": "2@BTC", "domain": 2},
"dst": {
"address": self.contract_address_btc,
"amount": "2@BTC",
"domain": 3,
},
}
]
)
self.node.generate(1)

assert_equal(
self.btc.functions.balanceOf(self.contract_address_btc).call()
/ math.pow(10, self.btc.functions.decimals().call()),
Decimal(2),
)
assert_equal(
self.btc.functions.totalSupply().call()
/ math.pow(10, self.btc.functions.decimals().call()),
Decimal(5.5),
)

def test_negative_transfer(self):
assert_raises_rpc_error(
-3,
Expand Down Expand Up @@ -869,7 +843,6 @@ def run_test(self):
self.test_multiple_dvm_evm_bridge()
self.test_conflicting_bridge()
self.test_invalid_token()
self.test_transfer_to_token_address()
self.test_bridge_when_no_balance()
self.test_negative_transfer()
self.test_different_tokens()
Expand Down
45 changes: 0 additions & 45 deletions test/functional/feature_evm_transferdomain.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
# Copyright (c) DeFi Blockchain Developers
# Distributed under the MIT software license, see the accompanying
# file LICENSE or http://www.opensource.org/licenses/mit-license.php.
import web3.utils

from test_framework.test_framework import DefiTestFramework
from test_framework.util import assert_equal, assert_raises_rpc_error, int_to_eth_u256
Expand Down Expand Up @@ -761,49 +760,6 @@ def invalid_transfer_sc(self):
],
)

def invalid_transfer_sc_mempool(self):
abi, bytecode, _ = EVMContract.from_file("Reverter.sol", "Reverter").compile()
compiled = self.nodes[0].w3.eth.contract(abi=abi, bytecode=bytecode)
nonce = self.nodes[0].w3.eth.get_transaction_count(self.evm_key_pair.address)

tx = compiled.constructor().build_transaction(
{
"chainId": self.nodes[0].w3.eth.chain_id,
"nonce": nonce,
"maxFeePerGas": 10_000_000_000,
"maxPriorityFeePerGas": 1_500_000_000,
"gas": 1_000_000,
}
)
signed = self.nodes[0].w3.eth.account.sign_transaction(
tx, self.evm_key_pair.privkey
)
self.nodes[0].w3.eth.send_raw_transaction(signed.rawTransaction)

contract_address = web3.utils.get_create_address(
self.evm_key_pair.address, nonce
)

assert_raises_rpc_error(
-32600,
"EVM destination is a smart contract",
self.nodes[0].transferdomain,
[
{
"src": {
"address": self.address,
"amount": "1@DFI",
"domain": 2,
},
"dst": {
"address": contract_address,
"amount": "1@DFI",
"domain": 3,
},
}
],
)

def invalid_transfer_no_auth(self):
assert_raises_rpc_error(
-5,
Expand Down Expand Up @@ -1052,7 +1008,6 @@ def run_test(self):

# Transfer to smart contract
self.invalid_transfer_sc()
self.invalid_transfer_sc_mempool()

# Invalid authorisation
self.invalid_transfer_no_auth()
Expand Down

0 comments on commit a3145b1

Please sign in to comment.