From d9c88f7d87a1fe4ba8e0faaf0b244fcbb98ef2e6 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 544271eb175ed4..3e929921d485d4 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 bbcaa9ed2c70d4..2b16080b9f6080 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 40593988edb98e..12e8d3c7e02e5a 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 20440e23d6ea59..b93356f8b9d7aa 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