From 25b1c94811e1079b5a1584895d85835c3069aecc Mon Sep 17 00:00:00 2001
From: Dario Russi <113150618+dariorussi@users.noreply.github.com>
Date: Wed, 8 May 2024 18:32:47 +0200
Subject: [PATCH] audit feedback (#17573)
## Description
Audit feedback around `notional_value`, minor cleanup
## 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:
---
crates/sui-bridge/src/server/mod.rs | 2 +-
crates/sui-framework/docs/bridge/treasury.md | 70 ++++++++++++++-----
.../packages/bridge/sources/treasury.move | 62 +++++++++++-----
..._populated_genesis_snapshot_matches-2.snap | 28 ++++----
4 files changed, 113 insertions(+), 49 deletions(-)
diff --git a/crates/sui-bridge/src/server/mod.rs b/crates/sui-bridge/src/server/mod.rs
index 544271eb175ed..3e929921d485d 100644
--- a/crates/sui-bridge/src/server/mod.rs
+++ b/crates/sui-bridge/src/server/mod.rs
@@ -286,7 +286,7 @@ async fn handle_add_tokens_on_sui(
if !chain_id.is_sui_chain() {
return Err(BridgeError::InvalidBridgeClientRequest(
- "handle_add_tokens_on_evm only expects Sui chain id".to_string(),
+ "handle_add_tokens_on_sui only expects Sui chain id".to_string(),
));
}
diff --git a/crates/sui-framework/docs/bridge/treasury.md b/crates/sui-framework/docs/bridge/treasury.md
index bbcaa9ed2c70d..2b16080b9f608 100644
--- a/crates/sui-framework/docs/bridge/treasury.md
+++ b/crates/sui-framework/docs/bridge/treasury.md
@@ -299,11 +299,20 @@ title: Module `0xb::treasury`
## Constants
+
+
+
+
+const EInvalidNotionalValue: u64 = 4;
+
+
+
+
-const EInvalidUpgradeCap: u64 = 1;
+const EInvalidUpgradeCap: u64 = 2;
@@ -312,7 +321,7 @@ title: Module `0xb::treasury`
-const ETokenSupplyNonZero: u64 = 2;
+const ETokenSupplyNonZero: u64 = 3;
@@ -321,7 +330,7 @@ title: Module `0xb::treasury`
-const EUnsupportedTokenType: u64 = 0;
+const EUnsupportedTokenType: u64 = 1;
@@ -416,7 +425,12 @@ title: Module `0xb::treasury`
Implementation
-public(package) fun register_foreign_token<T>(self: &mut BridgeTreasury, tc: TreasuryCap<T>, uc: UpgradeCap, metadata: &CoinMetadata<T>) {
+public(package) fun register_foreign_token<T>(
+ self: &mut BridgeTreasury,
+ tc: TreasuryCap<T>,
+ uc: UpgradeCap,
+ metadata: &CoinMetadata<T>,
+) {
// Make sure TreasuryCap has not been minted before.
assert!(coin::total_supply(&tc) == 0, ETokenSupplyNonZero);
let type_name = type_name::get<T>();
@@ -424,14 +438,17 @@ title: Module `0xb::treasury`
let coin_address = address::from_bytes(address_bytes);
// Make sure upgrade cap is for the Coin package
// FIXME: add test
- assert!(object::id_to_address(&package::upgrade_package(&uc)) == coin_address, EInvalidUpgradeCap);
+ assert!(
+ object::id_to_address(&package::upgrade_package(&uc))
+ == coin_address, EInvalidUpgradeCap
+ );
let registration = ForeignTokenRegistration {
type_name,
uc,
decimal: coin::get_decimals(metadata),
};
- bag::add(&mut self.waiting_room, type_name::into_string(type_name), registration);
- object_bag::add(&mut self.treasuries, type_name, tc);
+ self.waiting_room.add(type_name::into_string(type_name), registration);
+ self.treasuries.add(type_name, tc);
emit(TokenRegistrationEvent{
type_name,
@@ -460,21 +477,31 @@ title: Module `0xb::treasury`
Implementation
-public(package) fun add_new_token(self: &mut BridgeTreasury, token_name: String, token_id:u8, native_token: bool, notional_value: u64) {
+public(package) fun add_new_token(
+ self: &mut BridgeTreasury,
+ token_name: String,
+ token_id:u8,
+ native_token: bool,
+ notional_value: u64,
+) {
if (!native_token){
+ assert!(notional_value > 0, EInvalidNotionalValue);
let ForeignTokenRegistration{
type_name,
uc,
decimal,
} = bag::remove<String, ForeignTokenRegistration>(&mut self.waiting_room, token_name);
let decimal_multiplier = math::pow(10, decimal);
- vec_map::insert(&mut self.supported_tokens, type_name, BridgeTokenMetadata{
- id: token_id,
- decimal_multiplier,
- notional_value,
- native_token
- });
- vec_map::insert(&mut self.id_token_type_map, token_id, type_name);
+ self.supported_tokens.insert(
+ type_name,
+ BridgeTokenMetadata{
+ id: token_id,
+ decimal_multiplier,
+ notional_value,
+ native_token
+ },
+ );
+ self.id_token_type_map.insert(token_id, type_name);
// Freeze upgrade cap to prevent changes to the coin
transfer::public_freeze_object(uc);
@@ -565,7 +592,11 @@ title: Module `0xb::treasury`
Implementation
-public(package) fun mint<T>(self: &mut BridgeTreasury, amount: u64, ctx: &mut TxContext): Coin<T> {
+public(package) fun mint<T>(
+ self: &mut BridgeTreasury,
+ amount: u64,
+ ctx: &mut TxContext,
+): Coin<T> {
let treasury = &mut self.treasuries[type_name::get<T>()];
coin::mint(treasury, amount, ctx)
}
@@ -590,9 +621,14 @@ title: Module `0xb::treasury`
Implementation
-public(package) fun update_asset_notional_price(self: &mut BridgeTreasury, token_id: u8, new_usd_price: u64) {
+public(package) fun update_asset_notional_price(
+ self: &mut BridgeTreasury,
+ token_id: u8,
+ new_usd_price: u64,
+) {
let type_name = self.id_token_type_map.try_get(&token_id);
assert!(type_name.is_some(), EUnsupportedTokenType);
+ assert!(new_usd_price > 0, EInvalidNotionalValue);
let type_name = type_name.destroy_some();
let metadata = self.supported_tokens.get_mut(&type_name);
metadata.notional_value = new_usd_price;
diff --git a/crates/sui-framework/packages/bridge/sources/treasury.move b/crates/sui-framework/packages/bridge/sources/treasury.move
index 40593988edb98..12e8d3c7e02e5 100644
--- a/crates/sui-framework/packages/bridge/sources/treasury.move
+++ b/crates/sui-framework/packages/bridge/sources/treasury.move
@@ -20,9 +20,10 @@ module bridge::treasury {
use sui::vec_map;
use sui::vec_map::VecMap;
- const EUnsupportedTokenType: u64 = 0;
- const EInvalidUpgradeCap: u64 = 1;
- const ETokenSupplyNonZero: u64 = 2;
+ const EUnsupportedTokenType: u64 = 1;
+ const EInvalidUpgradeCap: u64 = 2;
+ const ETokenSupplyNonZero: u64 = 3;
+ const EInvalidNotionalValue: u64 = 4;
#[test_only]
const USD_VALUE_MULTIPLIER: u64 = 100000000; // 8 DP accuracy
@@ -92,7 +93,12 @@ module bridge::treasury {
// Internal functions
//
- public(package) fun register_foreign_token(self: &mut BridgeTreasury, tc: TreasuryCap, uc: UpgradeCap, metadata: &CoinMetadata) {
+ public(package) fun register_foreign_token(
+ self: &mut BridgeTreasury,
+ tc: TreasuryCap,
+ uc: UpgradeCap,
+ metadata: &CoinMetadata,
+ ) {
// Make sure TreasuryCap has not been minted before.
assert!(coin::total_supply(&tc) == 0, ETokenSupplyNonZero);
let type_name = type_name::get();
@@ -100,14 +106,17 @@ module bridge::treasury {
let coin_address = address::from_bytes(address_bytes);
// Make sure upgrade cap is for the Coin package
// FIXME: add test
- assert!(object::id_to_address(&package::upgrade_package(&uc)) == coin_address, EInvalidUpgradeCap);
+ assert!(
+ object::id_to_address(&package::upgrade_package(&uc))
+ == coin_address, EInvalidUpgradeCap
+ );
let registration = ForeignTokenRegistration {
type_name,
uc,
decimal: coin::get_decimals(metadata),
};
- bag::add(&mut self.waiting_room, type_name::into_string(type_name), registration);
- object_bag::add(&mut self.treasuries, type_name, tc);
+ self.waiting_room.add(type_name::into_string(type_name), registration);
+ self.treasuries.add(type_name, tc);
emit(TokenRegistrationEvent{
type_name,
@@ -116,21 +125,31 @@ module bridge::treasury {
});
}
- public(package) fun add_new_token(self: &mut BridgeTreasury, token_name: String, token_id:u8, native_token: bool, notional_value: u64) {
+ public(package) fun add_new_token(
+ self: &mut BridgeTreasury,
+ token_name: String,
+ token_id:u8,
+ native_token: bool,
+ notional_value: u64,
+ ) {
if (!native_token){
+ assert!(notional_value > 0, EInvalidNotionalValue);
let ForeignTokenRegistration{
type_name,
uc,
decimal,
} = bag::remove(&mut self.waiting_room, token_name);
let decimal_multiplier = math::pow(10, decimal);
- vec_map::insert(&mut self.supported_tokens, type_name, BridgeTokenMetadata{
- id: token_id,
- decimal_multiplier,
- notional_value,
- native_token
- });
- vec_map::insert(&mut self.id_token_type_map, token_id, type_name);
+ self.supported_tokens.insert(
+ type_name,
+ BridgeTokenMetadata{
+ id: token_id,
+ decimal_multiplier,
+ notional_value,
+ native_token
+ },
+ );
+ self.id_token_type_map.insert(token_id, type_name);
// Freeze upgrade cap to prevent changes to the coin
transfer::public_freeze_object(uc);
@@ -161,14 +180,23 @@ module bridge::treasury {
coin::burn(treasury, token);
}
- public(package) fun mint(self: &mut BridgeTreasury, amount: u64, ctx: &mut TxContext): Coin {
+ public(package) fun mint(
+ self: &mut BridgeTreasury,
+ amount: u64,
+ ctx: &mut TxContext,
+ ): Coin {
let treasury = &mut self.treasuries[type_name::get()];
coin::mint(treasury, amount, ctx)
}
- public(package) fun update_asset_notional_price(self: &mut BridgeTreasury, token_id: u8, new_usd_price: u64) {
+ public(package) fun update_asset_notional_price(
+ self: &mut BridgeTreasury,
+ token_id: u8,
+ new_usd_price: u64,
+ ) {
let type_name = self.id_token_type_map.try_get(&token_id);
assert!(type_name.is_some(), EUnsupportedTokenType);
+ assert!(new_usd_price > 0, EInvalidNotionalValue);
let type_name = type_name.destroy_some();
let metadata = self.supported_tokens.get_mut(&type_name);
metadata.notional_value = new_usd_price;
diff --git a/crates/sui-swarm-config/tests/snapshots/snapshot_tests__populated_genesis_snapshot_matches-2.snap b/crates/sui-swarm-config/tests/snapshots/snapshot_tests__populated_genesis_snapshot_matches-2.snap
index 20440e23d6ea5..b93356f8b9d7a 100644
--- a/crates/sui-swarm-config/tests/snapshots/snapshot_tests__populated_genesis_snapshot_matches-2.snap
+++ b/crates/sui-swarm-config/tests/snapshots/snapshot_tests__populated_genesis_snapshot_matches-2.snap
@@ -240,13 +240,13 @@ validators:
next_epoch_worker_address: ~
extra_fields:
id:
- id: "0xe9a924e5ac20e62c9b7e4eda76d84caa80c82ece4d67365cd66831aaff2b3d2c"
+ id: "0x92fa848f77ca565825d4d7a6dc011bf5523011c68d4e76affa862b6d11531e70"
size: 0
voting_power: 10000
- operation_cap_id: "0xebd58ebbd24454f7dee0951f59756d5dff8a036a98ef649448b4117220e579a8"
+ operation_cap_id: "0x3cd506993f5f137ffb871e636912ca2b69a057e518f068c6ad50a49817f9699d"
gas_price: 1000
staking_pool:
- id: "0x43f722383ed1307f79a77221d3552f8c6bafb338e6b1819dba853a215775f8f2"
+ id: "0x3bb0c293a1bc53e650d7dd56dfb556dd91544fbbb219b8655dc7a8c74c24a738"
activation_epoch: 0
deactivation_epoch: ~
sui_balance: 20000000000000000
@@ -254,14 +254,14 @@ validators:
value: 0
pool_token_balance: 20000000000000000
exchange_rates:
- id: "0x02a21ff127e6711994c01bff7aa4ac4ec7b8676a4af4bf5d09572b2e0fbdec11"
+ id: "0x07e5afd44056b0238e0a86a35385263cb07eb773e175ae6a8c741601f63258ba"
size: 1
pending_stake: 0
pending_total_sui_withdraw: 0
pending_pool_token_withdraw: 0
extra_fields:
id:
- id: "0x2d2d9310af95329f896b0697cad5629ea5229602e237154b544082b3c529746a"
+ id: "0x78cccd9d80b5bad4b299ea31d9658f29ee7d914eb35078f246654b305d66e57c"
size: 0
commission_rate: 200
next_epoch_stake: 20000000000000000
@@ -269,27 +269,27 @@ validators:
next_epoch_commission_rate: 200
extra_fields:
id:
- id: "0xe4c598c8f1b8d43b5c3bfa44d884bbf3e141adf9bb842368e3d57e4ccffc6d78"
+ id: "0xa98f56056109cb6b515c6930d0bf91220e531461238832b6ad217394deccc9c0"
size: 0
pending_active_validators:
contents:
- id: "0x73438e46e7b16e59baae371bab9dd684e991516a72de27bb4cdbd58c0fbfab28"
+ id: "0x5ac8304a67f3318514c07ef4eac2e87a561b588119143989e232f696e4d46cd9"
size: 0
pending_removals: []
staking_pool_mappings:
- id: "0x4014984e55dd491274f010cb72d6b11b9d68018cffc177d61bd314d5056e15c2"
+ id: "0xfee70cecca0b27d4a4f6f302713a6723459b97dfdadd99c39d7e47cb64fec588"
size: 1
inactive_validators:
- id: "0x2e70f2707d90bc2aacb0f46fe62371eba1145ce00890a52eeb4d9df5dbce2771"
+ id: "0xa3242a6fbf153af9968114acdf3c814bcf89a19558fb00160832e05a7642aa8a"
size: 0
validator_candidates:
- id: "0x99bd8e3fd0f475811588d89f33fd8d19bfa46bfc2143dd274c236c236bea71ec"
+ id: "0x03777a241718433e6382131190141e5d810ff1630f3aba8abc03bbf621b2cda1"
size: 0
at_risk_validators:
contents: []
extra_fields:
id:
- id: "0x46c784bb46270e80ef4703f24f9a00c6b37a46254d16900cb0674ccaf2056c78"
+ id: "0xfbb1fd2266e81b634ec9dbc6ee684e06436aeaf0a4d526580faed3a18ce4e8f0"
size: 0
storage_fund:
total_object_storage_rebates:
@@ -306,7 +306,7 @@ parameters:
validator_low_stake_grace_period: 7
extra_fields:
id:
- id: "0xa649cdfcbc60a7e2efcf0a311b01df067d2893c3752a5cba01d035aa4430e855"
+ id: "0x35cb023a4b3802f3c7aa87f31c17eb299882ca2a7c3a2a29eda2a7dcf43034ea"
size: 0
reference_gas_price: 1000
validator_report_records:
@@ -320,7 +320,7 @@ stake_subsidy:
stake_subsidy_decrease_rate: 1000
extra_fields:
id:
- id: "0x54fccd1879770d9b43d1d94270bd607547ec0fc64ef93aeb9341b8bf20938c48"
+ id: "0x3e3ec3feb7fa8aaee0dfb632aafdd6be9f2497eeebbb6a0297d92711f71d3717"
size: 0
safe_mode: false
safe_mode_storage_rewards:
@@ -332,6 +332,6 @@ safe_mode_non_refundable_storage_fee: 0
epoch_start_timestamp_ms: 10
extra_fields:
id:
- id: "0xc996a89be387dce7f2abeb2a6bc4db0bfbbf96459b13e923dd468676c15a32ee"
+ id: "0xd05e924a53d4f7fa4465b19c89138b929a30f843d94216dc8ceaf211133cdaae"
size: 0