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.

Sign up for GitHub

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

[r2r] wait for EVM approval transaction confirmation #1706

Merged
merged 5 commits into from
Mar 13, 2023
Merged

[r2r] wait for EVM approval transaction confirmation #1706

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6671a23
Wait for the confirmation of the approval transaction for EVM tokens …
shamardy Mar 9, 2023
be6edaf
fix allowed amount check
shamardy Mar 10, 2023
04fcbdc
fix wait_for_approval_confirmation_until
shamardy Mar 10, 2023
b56f307
provide better error message for approve transaction confirmation fail
shamardy Mar 10, 2023
c28474b
use allowance call instead of waiting for approve transaction confirm…
shamardy Mar 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 39 additions & 20 deletions mm2src/coins/eth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2823,10 +2823,16 @@ impl EthCoin {
}

fn send_hash_time_locked_payment(&self, args: SendPaymentArgs<'_>) -> EthTxFut {
const APPROVE_TX_CONFIRMATIONS: u64 = 1;
const APPROVE_TX_CHECK_EVERY: u64 = 5;

let receiver_addr = try_tx_fus!(addr_from_raw_pubkey(args.other_pubkey));
let swap_contract_address = try_tx_fus!(args.swap_contract_address.try_to_address());
let id = self.etomic_swap_id(args.time_lock, args.secret_hash);
let trade_amount = try_tx_fus!(wei_from_big_decimal(&args.amount, self.decimals));
let watcher_reward = args.watcher_reward.map(U256::from);
let time_lock = U256::from(args.time_lock);
let gas = U256::from(ETH_GAS);

let secret_hash = if args.secret_hash.len() == 32 {
ripemd160(args.secret_hash).to_vec()
Expand All @@ -2836,31 +2842,32 @@ impl EthCoin {

match &self.coin_type {
EthCoinType::Eth => {
let function_name = get_function_name("ethPayment", args.watcher_reward.is_some());
let function_name = get_function_name("ethPayment", watcher_reward.is_some());
let function = try_tx_fus!(SWAP_CONTRACT.function(&function_name));
let mut value = trade_amount;

let data = match args.watcher_reward {
let mut value = trade_amount;
let data = match watcher_reward {
Some(reward) => {
value += U256::from(reward);
value += reward;

try_tx_fus!(function.encode_input(&[
Token::FixedBytes(id),
Token::Address(receiver_addr),
Token::FixedBytes(secret_hash),
Token::Uint(U256::from(args.time_lock)),
Token::Uint(U256::from(reward)),
Token::Uint(time_lock),
Token::Uint(reward),
]))
},
None => try_tx_fus!(function.encode_input(&[
Token::FixedBytes(id),
Token::Address(receiver_addr),
Token::FixedBytes(secret_hash),
Token::Uint(U256::from(args.time_lock)),
Token::Uint(time_lock),
])),
};
drop_mutability!(value);

self.sign_and_send_transaction(value, Action::Call(swap_contract_address), data, U256::from(ETH_GAS))
self.sign_and_send_transaction(value, Action::Call(swap_contract_address), data, gas)
},
EthCoinType::Erc20 {
platform: _,
Expand All @@ -2870,7 +2877,7 @@ impl EthCoin {
.allowance(swap_contract_address)
.map_err(|e| TransactionErr::Plain(ERRL!("{}", e)));

let function_name = get_function_name("erc20Payment", args.watcher_reward.is_some());
let function_name = get_function_name("erc20Payment", watcher_reward.is_some());
let function = try_tx_fus!(SWAP_CONTRACT.function(&function_name));

let data = try_tx_fus!(function.encode_input(&[
Expand All @@ -2879,30 +2886,42 @@ impl EthCoin {
Token::Address(*token_addr),
Token::Address(receiver_addr),
Token::FixedBytes(secret_hash),
Token::Uint(U256::from(args.time_lock))
Token::Uint(time_lock)
]));
let value = U256::from(args.watcher_reward.unwrap_or(0));
// wait for the approval tx confirmation only half the time required for the actual htlc tx confirmation
let wait_for_approval_confirmation_until = args.wait_for_confirmation_until / 2;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@caglaryucekaya Do you think the wait period should be reduced for the approval confirmation? I don't want the check for confirmation to fail quickly if there is no new blocks for some time but if there is a problem with the approval transaction we want it to fail quickly, so that is the trade off between longer vs shorter wait time.

Copy link

Choose a reason for hiding this comment

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

We could put some random limit on it, but I think we can leave it this way. If the transaction is sent successfully with enough gas and correct addresses it shouldn't fail. Are there any other reasons why it can fail?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

but I think we can leave it this way

Yeah, I think so too.

Are there any other reasons why it can fail?

There is a more general problem with block reorganization specially with polygon, but this should be solved using a higher value for required confs for both taker and maker and if the approve transaction disappears, dependant transactions will fail too, so I see no problem with the current approach.


let arc = self.clone();
Box::new(allowance_fut.and_then(move |allowed| -> EthTxFut {
if allowed < value {
if allowed < trade_amount {
Box::new(
arc.approve(swap_contract_address, U256::max_value())
.and_then(move |_approved| {
arc.sign_and_send_transaction(
value,
Action::Call(swap_contract_address),
data,
U256::from(ETH_GAS),
.and_then(move |approved| {
// make sure the approve tx is confirmed before sending the htlc tx
arc.wait_for_confirmations(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I decided to wait for confirmation of the approve transaction, alternatively, we can loop over the allowance fn and check until the amount is allowed. What do you think is the better solution @caglaryucekaya? checking allowance fn might be a cheaper call but I am not sure.

Copy link

@caglaryucekaya caglaryucekaya Mar 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

https://www.quicknode.com/docs/ethereum/api_credits

Quicknode has an API credit measure here for each method based on its complexity. wait_for_confirmations requires two calls at each iteration of the loop: eth_getTransactionReceipt (2 credits) and eth_blockNumber (1 credit). Allowance query requires one call eth_call which is 2 credits. So allowance call is less credits in total according to this list, and one call is better than having two separate calls in general. I think it's worth changing it to a loop over the allowance function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the detailed research/explanation for this. Will fix it :)

&BytesJson::from(approved.tx_hex()),
APPROVE_TX_CONFIRMATIONS,
false,
wait_for_approval_confirmation_until,
APPROVE_TX_CHECK_EVERY,
)
.map_err(TransactionErr::Plain)
.and_then(move |_confirmed| {
arc.sign_and_send_transaction(
watcher_reward.unwrap_or_else(|| 0.into()),
Action::Call(swap_contract_address),
data,
gas,
)
})
}),
)
} else {
Box::new(arc.sign_and_send_transaction(
value,
watcher_reward.unwrap_or_else(|| 0.into()),
Action::Call(swap_contract_address),
data,
U256::from(ETH_GAS),
gas,
))
}
}))
Expand Down
2 changes: 2 additions & 0 deletions mm2src/coins/eth/eth_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ fn send_and_refund_erc20_payment() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let payment = coin.send_maker_payment(maker_payment_args).wait().unwrap();
log!("{:?}", payment);
Expand Down Expand Up @@ -332,6 +333,7 @@ fn send_and_refund_eth_payment() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let payment = coin.send_maker_payment(send_maker_payment_args).wait().unwrap();

Expand Down
1 change: 1 addition & 0 deletions mm2src/coins/eth/eth_wasm_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ async fn test_send() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let tx = coin.send_maker_payment(maker_payment_args).compat().await;
console::log_1(&format!("{:?}", tx).into());
Expand Down
1 change: 1 addition & 0 deletions mm2src/coins/lp_coins.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,7 @@ pub struct SendPaymentArgs<'a> {
pub swap_unique_data: &'a [u8],
pub payment_instructions: &'a Option<PaymentInstructions>,
pub watcher_reward: Option<u64>,
pub wait_for_confirmation_until: u64,
}

#[derive(Clone, Debug)]
Expand Down
3 changes: 3 additions & 0 deletions mm2src/mm2_main/src/lp_swap/maker_swap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,8 @@ impl MakerSwap {
Ok(res) => match res {
Some(tx) => tx,
None => {
let maker_payment_wait_confirm =
wait_for_maker_payment_conf_until(self.r().data.started_at, self.r().data.lock_duration);
let payment_fut = self.maker_coin.send_maker_payment(SendPaymentArgs {
time_lock_duration: self.r().data.lock_duration,
time_lock: self.r().data.maker_payment_lock as u32,
Expand All @@ -838,6 +840,7 @@ impl MakerSwap {
swap_unique_data: &unique_data,
payment_instructions: &self.r().payment_instructions,
watcher_reward: reward_amount,
wait_for_confirmation_until: maker_payment_wait_confirm,
});

match payment_fut.compat().await {
Expand Down
1 change: 1 addition & 0 deletions mm2src/mm2_main/src/lp_swap/taker_swap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1453,6 +1453,7 @@ impl TakerSwap {
swap_unique_data: &unique_data,
payment_instructions: &self.r().payment_instructions,
watcher_reward: reward_amount,
wait_for_confirmation_until: self.r().data.taker_payment_lock,
});

match payment_fut.compat().await {
Expand Down
5 changes: 5 additions & 0 deletions mm2src/mm2_main/tests/docker_tests/docker_tests_inner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ fn test_search_for_swap_tx_spend_native_was_refunded_taker() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let tx = coin.send_taker_payment(taker_payment_args).wait().unwrap();

Expand Down Expand Up @@ -111,6 +112,7 @@ fn test_search_for_swap_tx_spend_native_was_refunded_maker() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let tx = coin.send_maker_payment(maker_payment_args).wait().unwrap();

Expand Down Expand Up @@ -170,6 +172,7 @@ fn test_search_for_taker_swap_tx_spend_native_was_spent_by_maker() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let tx = coin.send_taker_payment(taker_payment_args).wait().unwrap();

Expand Down Expand Up @@ -230,6 +233,7 @@ fn test_search_for_maker_swap_tx_spend_native_was_spent_by_taker() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let tx = coin.send_maker_payment(maker_payment_args).wait().unwrap();

Expand Down Expand Up @@ -293,6 +297,7 @@ fn test_one_hundred_maker_payments_in_a_row_native() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let tx = coin.send_maker_payment(maker_payment_args).wait().unwrap();
if let TransactionEnum::UtxoTx(tx) = tx {
Expand Down
12 changes: 12 additions & 0 deletions mm2src/mm2_main/tests/docker_tests/qrc20_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ fn test_taker_spends_maker_payment() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let payment = maker_coin.send_maker_payment(maker_payment_args).wait().unwrap();
let payment_tx_hash = payment.tx_hash();
Expand Down Expand Up @@ -287,6 +288,7 @@ fn test_maker_spends_taker_payment() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let payment = taker_coin.send_taker_payment(taker_payment_args).wait().unwrap();
let payment_tx_hash = payment.tx_hash();
Expand Down Expand Up @@ -372,6 +374,7 @@ fn test_maker_refunds_payment() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let payment = coin.send_maker_payment(maker_payment).wait().unwrap();
let payment_tx_hash = payment.tx_hash();
Expand Down Expand Up @@ -434,6 +437,7 @@ fn test_taker_refunds_payment() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let payment = coin.send_taker_payment(taker_payment_args).wait().unwrap();
let payment_tx_hash = payment.tx_hash();
Expand Down Expand Up @@ -493,6 +497,7 @@ fn test_check_if_my_payment_sent() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let payment = coin.send_maker_payment(maker_payment_args).wait().unwrap();
let payment_tx_hash = payment.tx_hash();
Expand Down Expand Up @@ -544,6 +549,7 @@ fn test_search_for_swap_tx_spend_taker_spent() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let payment = maker_coin.send_maker_payment(maker_payment_args).wait().unwrap();
let payment_tx_hash = payment.tx_hash();
Expand Down Expand Up @@ -617,6 +623,7 @@ fn test_search_for_swap_tx_spend_maker_refunded() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let payment = maker_coin.send_maker_payment(maker_payment_args).wait().unwrap();
let payment_tx_hash = payment.tx_hash();
Expand Down Expand Up @@ -689,6 +696,7 @@ fn test_search_for_swap_tx_spend_not_spent() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let payment = maker_coin.send_maker_payment(maker_payment_args).wait().unwrap();
let payment_tx_hash = payment.tx_hash();
Expand Down Expand Up @@ -741,6 +749,7 @@ fn test_wait_for_tx_spend() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let payment = maker_coin.send_maker_payment(maker_payment_args).wait().unwrap();
let payment_tx_hash = payment.tx_hash();
Expand Down Expand Up @@ -1058,6 +1067,7 @@ fn test_get_max_taker_vol_and_trade_with_dynamic_trade_fee(coin: QtumCoin, priv_
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};

let _taker_payment_tx = coin
Expand Down Expand Up @@ -1457,6 +1467,7 @@ fn test_search_for_segwit_swap_tx_spend_native_was_refunded_maker() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let tx = coin.send_maker_payment(maker_payment).wait().unwrap();

Expand Down Expand Up @@ -1515,6 +1526,7 @@ fn test_search_for_segwit_swap_tx_spend_native_was_refunded_taker() {
swap_unique_data: &[],
payment_instructions: &None,
watcher_reward: None,
wait_for_confirmation_until: 0,
};
let tx = coin.send_taker_payment(taker_payment).wait().unwrap();

Expand Down
Loading