Skip to content

Commit

Permalink
Feat(xcc): Use native bridge logic to unwrap wNEAR (#867)
Browse files Browse the repository at this point in the history 
## Description

Since #750 has been merged, the ERC-20 connector is able to unwrap
wrapped Near tokens into Near native tokens. This logic was duplicated
by the XCC router, but in this PR we remove the logic from XCC and use
the bridge's unwrapping instead. This has a nice side-effect of saving
us 20 Tgas from the XCC overhead.

## Performance / NEAR gas cost considerations

No impact on regular transactions, 20 Tgas improvement to XCC
transactions.

## Testing

Existing tests. Specifically this relies heavily on the upgrade test
introduced in #866
  • Loading branch information
@birchmd
birchmd authored Nov 17, 2023
1 parent f72aa84 commit a746806
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 82 deletions.
38 changes: 19 additions & 19 deletions engine/src/xcc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub const CODE_KEY: &[u8] = b"router_code";
pub const VERSION_UPDATE_GAS: NearGas = NearGas::new(5_000_000_000_000);
pub const INITIALIZE_GAS: NearGas = NearGas::new(15_000_000_000_000);
pub const UPGRADE_GAS: NearGas = NearGas::new(20_000_000_000_000);
pub const UNWRAP_AND_REFUND_GAS: NearGas = NearGas::new(25_000_000_000_000);
pub const REFUND_GAS: NearGas = NearGas::new(5_000_000_000_000);
pub const WITHDRAW_GAS: NearGas = NearGas::new(40_000_000_000_000);
/// Solidity selector for the `withdrawToNear` function
/// `https://www.4byte.directory/signatures/?bytes4_signature=0x6b351848`
Expand Down Expand Up @@ -312,22 +312,20 @@ pub fn handle_precompile_promise<I, P>(
AddressVersionStatus::DeployNeeded { create_needed } => create_needed,
AddressVersionStatus::UpToDate => false,
};
let args = format!(
r#"{{"amount": "{}", "refund_needed": {}}}"#,
required_near.as_u128(),
refund_needed,
);
let unwrap_call = PromiseCreateArgs {
target_account_id: promise.target_account_id.clone(),
method: "unwrap_and_refund_storage".into(),
args: args.into_bytes(),
attached_balance: ZERO_YOCTO,
attached_gas: UNWRAP_AND_REFUND_GAS,
};
// Safety: This call is safe because the router's `unwrap_and_refund_storage` method
// does not violate any security invariants. It only interacts with the wrap.near contract
// to obtain NEAR from WNEAR.
unsafe { Some(handler.promise_attach_callback(id, &unwrap_call)) }
if refund_needed {
let refund_call = PromiseCreateArgs {
target_account_id: promise.target_account_id.clone(),
method: "send_refund".into(),
args: Vec::new(),
attached_balance: ZERO_YOCTO,
attached_gas: REFUND_GAS,
};
// Safety: This call is safe because the router's `send_refund` method
// does not violate any security invariants. It only sends NEAR back to this contract.
unsafe { Some(handler.promise_attach_callback(id, &refund_call)) }
} else {
Some(id)
}
};
// 3. Finally we can do the call the user wanted to do.

Expand Down Expand Up @@ -457,8 +455,9 @@ impl AddressVersionStatus {
}

fn withdraw_to_near_args(recipient: &AccountId, amount: Yocto) -> Vec<u8> {
let recipient_with_msg = format!("{recipient}:unwrap");
let args = ethabi::encode(&[
ethabi::Token::Bytes(recipient.as_bytes().to_vec()),
ethabi::Token::Bytes(recipient_with_msg.into_bytes()),
ethabi::Token::Uint(U256::from(amount.as_u128())),
]);
[&WITHDRAW_TO_NEAR_SELECTOR, args.as_slice()].concat()
Expand Down Expand Up @@ -537,6 +536,7 @@ mod tests {
#[test]
fn test_withdraw_to_near_encoding() {
let recipient: AccountId = "some_account.near".parse().unwrap();
let recipient_with_msg = format!("{recipient}:unwrap");
let amount = Yocto::new(1332654);
#[allow(deprecated)]
let withdraw_function = ethabi::Function {
Expand All @@ -559,7 +559,7 @@ mod tests {
};
let expected_tx_data = withdraw_function
.encode_input(&[
ethabi::Token::Bytes(recipient.as_bytes().to_vec()),
ethabi::Token::Bytes(recipient_with_msg.into_bytes()),
ethabi::Token::Uint(U256::from(amount.as_u128())),
])
.unwrap();
Expand Down
130 changes: 67 additions & 63 deletions etc/xcc-router/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use aurora_engine_types::parameters::{
};
use near_sdk::borsh::{self, BorshDeserialize, BorshSerialize};
use near_sdk::collections::{LazyOption, LookupMap};
use near_sdk::json_types::{U128, U64};
use near_sdk::json_types::U64;
use near_sdk::BorshStorageKey;
use near_sdk::{
env, near_bindgen, AccountId, Gas, PanicOnDefault, Promise, PromiseIndex, PromiseResult,
Expand All @@ -27,15 +27,9 @@ const CURRENT_VERSION: u32 = std::include!("VERSION");

const ERR_ILLEGAL_CALLER: &str = "ERR_ILLEGAL_CALLER";
const INITIALIZE_GAS: Gas = Gas(15_000_000_000_000);
/// Gas cost estimated from mainnet data. Cost seems to consistently be 3 Tgas, but we add a
/// little more to be safe. Example:
/// https://explorer.mainnet.near.org/transactions/3U9SKbGKM3MchLa2hLTNuYLdErcEDneJGbGv1cHZXuvE#HsHabUdJ7DRJcseNa4GQTYwm8KtbB4mqsq2AUssJWWv6
const WNEAR_WITHDRAW_GAS: Gas = Gas(5_000_000_000_000);
/// Gas cost estimated from mainnet data. Example:
/// https://explorer.mainnet.near.org/transactions/5NbZ7SfrodNxeLcSkCmLAEdbZfbkk9cjqz3zSDwktKrk#D7un3c3Nxv7Ee3JpQSKiM97LbwCDFPbMo5iLoijGPXPM
const WNEAR_REGISTER_GAS: Gas = Gas(5_000_000_000_000);
/// Gas cost estimated from simulation tests.
const REFUND_GAS: Gas = Gas(5_000_000_000_000);
/// Registration amount computed from FT token source code, see
/// https://github.com/near/near-sdk-rs/blob/master/near-contract-standards/src/fungible_token/core_impl.rs#L50
/// https://github.com/near/near-sdk-rs/blob/master/near-contract-standards/src/fungible_token/storage_impl.rs#L101
Expand Down Expand Up @@ -129,15 +123,15 @@ impl Router {
/// contract calls is used by the address associated with the sub-account this router contract
/// is deployed at.
pub fn execute(&self, #[serializer(borsh)] promise: PromiseArgs) {
self.require_parent_caller();
self.assert_preconditions();

let promise_id = Router::promise_create(promise);
env::promise_return(promise_id)
}

/// Similar security considerations here as for `execute`.
pub fn schedule(&mut self, #[serializer(borsh)] promise: PromiseArgs) {
self.require_parent_caller();
self.assert_preconditions();

let nonce = self.nonce.get().unwrap_or_default();
self.scheduled_promises.insert(&nonce, &promise);
Expand All @@ -160,42 +154,11 @@ impl Router {
env::promise_return(promise_id)
}

/// The router will receive wNEAR deposits from its user. This function is to
/// unwrap that wNEAR into NEAR. Additionally, this function will transfer some
/// NEAR back to its parent, if needed. This transfer is done because the parent
/// must cover the storage staking cost with the router account is first created,
/// but the user ultimately is responsible to pay for it.
pub fn unwrap_and_refund_storage(&self, amount: U128, refund_needed: bool) {
self.require_parent_caller();

let args = format!(r#"{{"amount": "{}"}}"#, amount.0);
let id = env::promise_create(
self.wnear_account.clone(),
"near_withdraw",
args.as_bytes(),
1,
WNEAR_WITHDRAW_GAS,
);
let final_id = if refund_needed {
env::promise_then(
id,
env::current_account_id(),
"send_refund",
&[],
0,
REFUND_GAS,
)
} else {
id
};
env::promise_return(final_id);
}

/// Allows the parent contract to trigger an update to the logic of this contract
/// (by deploying a new contract to this account);
#[payable]
pub fn deploy_upgrade(&mut self, #[serializer(borsh)] args: DeployUpgradeParams) {
self.require_parent_caller();
self.assert_preconditions();

let promise_id = env::promise_batch_create(&env::current_account_id());
env::promise_batch_action_deploy_contract(promise_id, &args.code);
Expand All @@ -209,35 +172,38 @@ impl Router {
env::promise_return(promise_id);
}

#[private]
pub fn send_refund(&self) -> Promise {
let parent = self
.parent
.get()
.unwrap_or_else(|| env::panic_str("ERR_CONTRACT_NOT_INITIALIZED"));
let parent = self.get_parent().unwrap_or_else(env_panic);

require_caller(&parent)
.and_then(|_| require_no_failed_promises())
.unwrap_or_else(env_panic);

Promise::new(parent).transfer(REFUND_AMOUNT)
}
}

impl Router {
fn require_parent_caller(&self) {
let caller = env::predecessor_account_id();
let parent = self
.parent
.get()
.unwrap_or_else(|| env::panic_str("ERR_CONTRACT_NOT_INITIALIZED"));
if caller != parent {
env::panic_str(ERR_ILLEGAL_CALLER);
}
// Any method that can only be called by the parent should also only be executed if
// the parent's execution was successful.
let num_promises = env::promise_results_count();
for index in 0..num_promises {
if let PromiseResult::Failed | PromiseResult::NotReady = env::promise_result(index) {
env::panic_str("ERR_CALLBACK_OF_FAILED_PROMISE");
}
}
fn get_parent(&self) -> Result<AccountId, Error> {
self.parent.get().ok_or(Error::ContractNotInitialized)
}

/// Checks the following preconditions:
/// 1. Contract is initialized
/// 2. predecessor_account_id == self.parent
/// 3. There are no failed promise results
/// These preconditions must be checked on methods where are important for
/// the security of the contract (e.g. `execute`).
fn require_preconditions(&self) -> Result<(), Error> {
let parent = self.get_parent()?;
require_caller(&parent)?;
require_no_failed_promises()?;
Ok(())
}

/// Panics if any of the preconditions checked in `require_preconditions` are not met.
fn assert_preconditions(&self) {
self.require_preconditions().unwrap_or_else(env_panic);
}

fn promise_create(promise: PromiseArgs) -> PromiseIndex {
Expand Down Expand Up @@ -399,3 +365,41 @@ fn to_sdk_pk(key: &aurora_engine_types::parameters::NearPublicKey) -> near_sdk::
// Unwrap should be safe because we only encode valid public keys
data.try_into().unwrap()
}

fn require_caller(caller: &AccountId) -> Result<(), Error> {
if caller != &env::predecessor_account_id() {
return Err(Error::IllegalCaller);
}
Ok(())
}

fn require_no_failed_promises() -> Result<(), Error> {
let num_promises = env::promise_results_count();
for index in 0..num_promises {
if let PromiseResult::Failed | PromiseResult::NotReady = env::promise_result(index) {
return Err(Error::CallbackOfFailedPromise);
}
}
Ok(())
}

fn env_panic<T>(e: Error) -> T {
env::panic_str(e.as_ref())
}

#[derive(Debug)]
enum Error {
ContractNotInitialized,
IllegalCaller,
CallbackOfFailedPromise,
}

impl AsRef<str> for Error {
fn as_ref(&self) -> &str {
match self {
Self::ContractNotInitialized => "ERR_CONTRACT_NOT_INITIALIZED",
Self::IllegalCaller => ERR_ILLEGAL_CALLER,
Self::CallbackOfFailedPromise => "ERR_CALLBACK_OF_FAILED_PROMISE",
}
}
}

0 comments on commit a746806

Please sign in to comment.