Skip to content

Commit

Permalink
[bridge] remove unnecessary &mut (#17470)
Browse files Browse the repository at this point in the history 
## Description

1. `get_token_transfer_action_signatures` and
`get_token_transfer_action_status` does not need mutable Bridge ref
2. switch the order of Bridge and clock in `claim_token_internal` so we
can use the receiver syntax

## Test plan

existing tests

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
  • Loading branch information
@longbowlu @patrickkuo
longbowlu authored and patrickkuo committed May 9, 2024
1 parent ed60cf4 commit 50f7110
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 23 deletions.
12 changes: 10 additions & 2 deletions crates/sui-bridge/src/action_executor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ use sui_types::{
transaction::Transaction,
};

use crate::events::{TokenTransferAlreadyClaimed, TokenTransferClaimed};
use crate::events::{
TokenTransferAlreadyApproved, TokenTransferAlreadyClaimed, TokenTransferApproved,
TokenTransferClaimed,
};
use crate::{
client::bridge_authority_aggregator::BridgeAuthorityAggregator,
error::BridgeError,
Expand Down Expand Up @@ -408,7 +411,12 @@ where
.data
.iter()
.any(|e| e.type_ == *TokenTransferAlreadyClaimed.get().unwrap()
|| e.type_ == *TokenTransferClaimed.get().unwrap()),);
|| e.type_ == *TokenTransferClaimed.get().unwrap()
|| e.type_ == *TokenTransferApproved.get().unwrap()
|| e.type_ == *TokenTransferAlreadyApproved.get().unwrap()),
"Expected TokenTransferAlreadyClaimed, TokenTransferClaimed, TokenTransferApproved or TokenTransferAlreadyApproved event but got: {:?}",
events,
);
info!(?tx_digest, "Sui transaction executed successfully");
store
.remove_pending_actions(&[action.digest()])
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-bridge/src/e2e_tests/basic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ async fn wait_for_transfer_action_status(
}
if now.elapsed().as_secs() > 30 {
panic!(
"Timeout waiting for token transfer action to be {:?}",
"Timeout waiting for token transfer action to be {:?}. chain_id: {chain_id}, nonce: {nonce}",
status
);
}
Expand Down
21 changes: 10 additions & 11 deletions crates/sui-framework/docs/bridge/bridge.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -969,9 +969,8 @@ title: Module `0xb::bridge`
bridge_seq_num: u64,
ctx: &<b>mut</b> TxContext,
): Coin&lt;T&gt; {
<b>let</b> (maybe_token, owner) = <a href="bridge.md#0xb_bridge_claim_token_internal">claim_token_internal</a>&lt;T&gt;(
<b>let</b> (maybe_token, owner) = <a href="bridge.md#0xb_bridge">bridge</a>.<a href="bridge.md#0xb_bridge_claim_token_internal">claim_token_internal</a>&lt;T&gt;(
<a href="../sui-framework/clock.md#0x2_clock">clock</a>,
<a href="bridge.md#0xb_bridge">bridge</a>,
source_chain,
bridge_seq_num,
ctx,
Expand Down Expand Up @@ -1009,7 +1008,7 @@ title: Module `0xb::bridge`
bridge_seq_num: u64,
ctx: &<b>mut</b> TxContext,
) {
<b>let</b> (token, owner) = <a href="bridge.md#0xb_bridge_claim_token_internal">claim_token_internal</a>&lt;T&gt;(<a href="../sui-framework/clock.md#0x2_clock">clock</a>, <a href="bridge.md#0xb_bridge">bridge</a>, source_chain, bridge_seq_num, ctx);
<b>let</b> (token, owner) = <a href="bridge.md#0xb_bridge">bridge</a>.<a href="bridge.md#0xb_bridge_claim_token_internal">claim_token_internal</a>&lt;T&gt;(<a href="../sui-framework/clock.md#0x2_clock">clock</a>, source_chain, bridge_seq_num, ctx);
<b>if</b> (token.is_some()) {
<a href="../sui-framework/transfer.md#0x2_transfer_public_transfer">transfer::public_transfer</a>(token.destroy_some(), owner)
} <b>else</b> {
Expand Down Expand Up @@ -1089,7 +1088,7 @@ title: Module `0xb::bridge`



<pre><code><b>fun</b> <a href="bridge.md#0xb_bridge_claim_token_internal">claim_token_internal</a>&lt;T&gt;(<a href="../sui-framework/clock.md#0x2_clock">clock</a>: &<a href="../sui-framework/clock.md#0x2_clock_Clock">clock::Clock</a>, <a href="bridge.md#0xb_bridge">bridge</a>: &<b>mut</b> <a href="bridge.md#0xb_bridge_Bridge">bridge::Bridge</a>, source_chain: u8, bridge_seq_num: u64, ctx: &<b>mut</b> <a href="../sui-framework/tx_context.md#0x2_tx_context_TxContext">tx_context::TxContext</a>): (<a href="../move-stdlib/option.md#0x1_option_Option">option::Option</a>&lt;<a href="../sui-framework/coin.md#0x2_coin_Coin">coin::Coin</a>&lt;T&gt;&gt;, <b>address</b>)
<pre><code><b>fun</b> <a href="bridge.md#0xb_bridge_claim_token_internal">claim_token_internal</a>&lt;T&gt;(<a href="bridge.md#0xb_bridge">bridge</a>: &<b>mut</b> <a href="bridge.md#0xb_bridge_Bridge">bridge::Bridge</a>, <a href="../sui-framework/clock.md#0x2_clock">clock</a>: &<a href="../sui-framework/clock.md#0x2_clock_Clock">clock::Clock</a>, source_chain: u8, bridge_seq_num: u64, ctx: &<b>mut</b> <a href="../sui-framework/tx_context.md#0x2_tx_context_TxContext">tx_context::TxContext</a>): (<a href="../move-stdlib/option.md#0x1_option_Option">option::Option</a>&lt;<a href="../sui-framework/coin.md#0x2_coin_Coin">coin::Coin</a>&lt;T&gt;&gt;, <b>address</b>)
</code></pre>


Expand All @@ -1099,8 +1098,8 @@ title: Module `0xb::bridge`


<pre><code><b>fun</b> <a href="bridge.md#0xb_bridge_claim_token_internal">claim_token_internal</a>&lt;T&gt;(
<a href="../sui-framework/clock.md#0x2_clock">clock</a>: &Clock,
<a href="bridge.md#0xb_bridge">bridge</a>: &<b>mut</b> <a href="bridge.md#0xb_bridge_Bridge">Bridge</a>,
<a href="../sui-framework/clock.md#0x2_clock">clock</a>: &Clock,
source_chain: u8,
bridge_seq_num: u64,
ctx: &<b>mut</b> TxContext,
Expand Down Expand Up @@ -1409,7 +1408,7 @@ title: Module `0xb::bridge`



<pre><code><b>public</b> <b>fun</b> <a href="bridge.md#0xb_bridge_get_token_transfer_action_status">get_token_transfer_action_status</a>(<a href="bridge.md#0xb_bridge">bridge</a>: &<b>mut</b> <a href="bridge.md#0xb_bridge_Bridge">bridge::Bridge</a>, source_chain: u8, bridge_seq_num: u64): u8
<pre><code><b>public</b> <b>fun</b> <a href="bridge.md#0xb_bridge_get_token_transfer_action_status">get_token_transfer_action_status</a>(<a href="bridge.md#0xb_bridge">bridge</a>: &<a href="bridge.md#0xb_bridge_Bridge">bridge::Bridge</a>, source_chain: u8, bridge_seq_num: u64): u8
</code></pre>


Expand All @@ -1419,11 +1418,11 @@ title: Module `0xb::bridge`


<pre><code><b>public</b> <b>fun</b> <a href="bridge.md#0xb_bridge_get_token_transfer_action_status">get_token_transfer_action_status</a>(
<a href="bridge.md#0xb_bridge">bridge</a>: &<b>mut</b> <a href="bridge.md#0xb_bridge_Bridge">Bridge</a>,
<a href="bridge.md#0xb_bridge">bridge</a>: &<a href="bridge.md#0xb_bridge_Bridge">Bridge</a>,
source_chain: u8,
bridge_seq_num: u64,
): u8 {
<b>let</b> inner = <a href="bridge.md#0xb_bridge_load_inner_mut">load_inner_mut</a>(<a href="bridge.md#0xb_bridge">bridge</a>);
<b>let</b> inner = <a href="bridge.md#0xb_bridge_load_inner">load_inner</a>(<a href="bridge.md#0xb_bridge">bridge</a>);
<b>let</b> key = <a href="message.md#0xb_message_create_key">message::create_key</a>(
source_chain,
<a href="message_types.md#0xb_message_types_token">message_types::token</a>(),
Expand Down Expand Up @@ -1457,7 +1456,7 @@ title: Module `0xb::bridge`



<pre><code><b>fun</b> <a href="bridge.md#0xb_bridge_get_token_transfer_action_signatures">get_token_transfer_action_signatures</a>(self: &<b>mut</b> <a href="bridge.md#0xb_bridge_Bridge">bridge::Bridge</a>, source_chain: u8, bridge_seq_num: u64): <a href="../move-stdlib/option.md#0x1_option_Option">option::Option</a>&lt;<a href="../move-stdlib/vector.md#0x1_vector">vector</a>&lt;<a href="../move-stdlib/vector.md#0x1_vector">vector</a>&lt;u8&gt;&gt;&gt;
<pre><code><b>fun</b> <a href="bridge.md#0xb_bridge_get_token_transfer_action_signatures">get_token_transfer_action_signatures</a>(<a href="bridge.md#0xb_bridge">bridge</a>: &<a href="bridge.md#0xb_bridge_Bridge">bridge::Bridge</a>, source_chain: u8, bridge_seq_num: u64): <a href="../move-stdlib/option.md#0x1_option_Option">option::Option</a>&lt;<a href="../move-stdlib/vector.md#0x1_vector">vector</a>&lt;<a href="../move-stdlib/vector.md#0x1_vector">vector</a>&lt;u8&gt;&gt;&gt;
</code></pre>


Expand All @@ -1467,11 +1466,11 @@ title: Module `0xb::bridge`


<pre><code><b>fun</b> <a href="bridge.md#0xb_bridge_get_token_transfer_action_signatures">get_token_transfer_action_signatures</a>(
self: &<b>mut</b> <a href="bridge.md#0xb_bridge_Bridge">Bridge</a>,
<a href="bridge.md#0xb_bridge">bridge</a>: &<a href="bridge.md#0xb_bridge_Bridge">Bridge</a>,
source_chain: u8,
bridge_seq_num: u64,
): Option&lt;<a href="../move-stdlib/vector.md#0x1_vector">vector</a>&lt;<a href="../move-stdlib/vector.md#0x1_vector">vector</a>&lt;u8&gt;&gt;&gt; {
<b>let</b> inner = <a href="bridge.md#0xb_bridge_load_inner_mut">load_inner_mut</a>(self);
<b>let</b> inner = <a href="bridge.md#0xb_bridge_load_inner">load_inner</a>(<a href="bridge.md#0xb_bridge">bridge</a>);
<b>let</b> key = <a href="message.md#0xb_message_create_key">message::create_key</a>(
source_chain,
<a href="message_types.md#0xb_message_types_token">message_types::token</a>(),
Expand Down
16 changes: 7 additions & 9 deletions crates/sui-framework/packages/bridge/sources/bridge.move
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,8 @@ module bridge::bridge {
bridge_seq_num: u64,
ctx: &mut TxContext,
): Coin<T> {
let (maybe_token, owner) = claim_token_internal<T>(
let (maybe_token, owner) = bridge.claim_token_internal<T>(
clock,
bridge,
source_chain,
bridge_seq_num,
ctx,
Expand All @@ -322,7 +321,7 @@ module bridge::bridge {
bridge_seq_num: u64,
ctx: &mut TxContext,
) {
let (token, owner) = claim_token_internal<T>(clock, bridge, source_chain, bridge_seq_num, ctx);
let (token, owner) = bridge.claim_token_internal<T>(clock, source_chain, bridge_seq_num, ctx);
if (token.is_some()) {
transfer::public_transfer(token.destroy_some(), owner)
} else {
Expand All @@ -339,7 +338,6 @@ module bridge::bridge {
inner
}

#[allow(unused_function)] // TODO: remove annotation after implementing user-facing API
fun load_inner(
bridge: &Bridge,
): &BridgeInner {
Expand All @@ -355,8 +353,8 @@ module bridge::bridge {
// Claim token from approved bridge message
// Returns Some(Coin) if coin can be claimed. If already claimed, return None
fun claim_token_internal<T>(
clock: &Clock,
bridge: &mut Bridge,
clock: &Clock,
source_chain: u8,
bridge_seq_num: u64,
ctx: &mut TxContext,
Expand Down Expand Up @@ -537,11 +535,11 @@ module bridge::bridge {
}

public fun get_token_transfer_action_status(
bridge: &mut Bridge,
bridge: &Bridge,
source_chain: u8,
bridge_seq_num: u64,
): u8 {
let inner = load_inner_mut(bridge);
let inner = load_inner(bridge);
let key = message::create_key(
source_chain,
message_types::token(),
Expand All @@ -566,11 +564,11 @@ module bridge::bridge {

#[allow(unused_function)]
fun get_token_transfer_action_signatures(
self: &mut Bridge,
bridge: &Bridge,
source_chain: u8,
bridge_seq_num: u64,
): Option<vector<vector<u8>>> {
let inner = load_inner_mut(self);
let inner = load_inner(bridge);
let key = message::create_key(
source_chain,
message_types::token(),
Expand Down

0 comments on commit 50f7110

Please sign in to comment.