feat: preparing for async method calls compatibility #105
Conversation
|
@kiseln I assume that we don't need/can't specify nonce for solana. Correct me if I'm wrong P.S: I saw that it uses latest hash, but that means that the only way to make sure that tx won't be replayed with same signature is to process them sequentially? It won't be too long, since finalization time is short, but it's still a bit weird 
|
|
@frolvanya transaction with the same hash will not be processed on Solana. It doesn't look like we have a problem with concurrency there since hash will change depending on the transfer payload |
bridge-cli/Cargo.toml
Outdated
| @@ -1,6 +1,6 @@ | |||
| [package] | |||
| name = "bridge-cli" | |||
| version = "0.2.2" | |||
| version = "0.2.4" | |||
There was a problem hiding this comment.
I just synced with main. I don't know why it shows a diff here
There was a problem hiding this comment.
But it's a good idea to prepare next version for this PR: 30533e1
| pub async fn fin_transfer( | ||
| &self, | ||
| transfer_log: OmniBridgeEvent, | ||
| nonce: Option<U256>, |
There was a problem hiding this comment.
It's been confusing from the first glance that this nonce is a transaction nonce as opposed to the transfer. Since this term is used in multiple domain, should we rename it to tx_nonce everywhere for clarity?
| @@ -396,13 +426,16 @@ impl NearBridgeClient { | |||
| transfer_id: TransferId, | |||
| fee_recipient: Option<AccountId>, | |||
| fee: Option<Fee>, | |||
| nonce: Option<u64>, | |||
| wait_until: TxExecutionStatus, | |||
There was a problem hiding this comment.
Does it make sense to combine nonce and wait_until in a separate struct (TransactionOptions?) to separate them from domain-specific arguments
There was a problem hiding this comment.
Good idea. Fixed in eb65f3f
Related to Near-One/omni-bridge#238
Also, for bridge-cli I decided to keep it simple:
wait_untilwas set toIncludedsince it's the fastest option and it would show tx_hash asap, so user could check it in explorerP.S: A bit more context why specifying nonce is crucial:
Right now relayer tries to process all events at the same time, so it results in creating different transactions with the same nonce, so only one tx would be processed. To resolve this we can either process each tx sequentially and wait for
Finalstatus or by having an atomic variable that would be our nonce for each chain (near, eth, base and arb) to avoid collisions. First case is very slow, so @karim-en proposed to go with a second solution