Merge branch 'master' into standardize-token-usage

.github/actions/bazel-test-all/action.yaml

@@ -24,7 +24,7 @@ runs:
 
           # freshly deployed k8s machines require ownership correctly set
           if [ -e /cache ]; then
-            sudo chown -RL 1001:1001 /cache
+            sudo find /cache -not -user 1001 -or -not -group 1001 -exec chown 1001:1001 {} +
           fi
 
           if [ -n "$SSH_PRIVATE_KEY_BACKUP_POD" ]; then

.github/workflows-source/schedule-hourly.yml

@@ -62,9 +62,11 @@ jobs:
         env:
           CLOUD_CREDENTIALS_CONTENT: ${{ secrets.CLOUD_CREDENTIALS_CONTENT }}
         with:
+          # --config=stamped is required for the BNs as they rely for both dev and prod deployment
+          # on images being uploaded to the S3 bucket
           BAZEL_COMMAND: >-
             build
-              --config=ci
+              --config=ci --config=stamped
               --repository_cache= --disk_cache= --noremote_accept_cached --remote_instance_name=${CI_COMMIT_SHA} --@rules_rust//rust/settings:pipelined_compilation=True
           BAZEL_TARGETS: //...
           GPG_PASSPHRASE: ${{ secrets.GPG_PASSPHRASE }}
@@ -83,8 +85,10 @@ jobs:
         id: bazel-test-all
         uses:  ./.github/actions/bazel-test-all/
         with:
+          # --config=stamped is required for the BNs as they rely for both dev and prod deployment
+          # on images being uploaded to the S3 bucket
           BAZEL_COMMAND: >-
-            test --config=ci --keep_going --test_tag_filters=system_test_hourly
+            test --config=ci --config=stamped --keep_going --test_tag_filters=system_test_hourly
           BAZEL_TARGETS: //rs/...
           BUILDEVENT_APIKEY: ${{ secrets.HONEYCOMB_TOKEN }}
           GPG_PASSPHRASE: ${{ secrets.GPG_PASSPHRASE }}

.github/workflows/schedule-hourly.yml

@@ -33,11 +33,13 @@ jobs:
         env:
           CLOUD_CREDENTIALS_CONTENT: ${{ secrets.CLOUD_CREDENTIALS_CONTENT }}
         with:
+          # --config=stamped is required for the BNs as they rely for both dev and prod deployment
+          # on images being uploaded to the S3 bucket
           BAZEL_COMMAND: >-
             build
 
 
-              --config=ci
+              --config=ci --config=stamped
               --repository_cache= --disk_cache= --noremote_accept_cached --remote_instance_name=${CI_COMMIT_SHA} --@rules_rust//rust/settings:pipelined_compilation=True
           BAZEL_TARGETS: //...
           GPG_PASSPHRASE: ${{ secrets.GPG_PASSPHRASE }}
@@ -76,8 +78,10 @@ jobs:
         id: bazel-test-all
         uses: ./.github/actions/bazel-test-all/
         with:
+          # --config=stamped is required for the BNs as they rely for both dev and prod deployment
+          # on images being uploaded to the S3 bucket
           BAZEL_COMMAND: >-
-            test --config=ci --keep_going --test_tag_filters=system_test_hourly
+            test --config=ci --config=stamped --keep_going --test_tag_filters=system_test_hourly
           BAZEL_TARGETS: //rs/...
           BUILDEVENT_APIKEY: ${{ secrets.HONEYCOMB_TOKEN }}
           GPG_PASSPHRASE: ${{ secrets.GPG_PASSPHRASE }}

rs/ledger_suite/icrc1/ledger/src/lib.rs

@@ -68,6 +68,11 @@ const MAX_TRANSACTIONS_TO_PURGE: usize = 100_000;
 #[allow(dead_code)]
 const MAX_U64_ENCODING_BYTES: usize = 10;
 const DEFAULT_MAX_MEMO_LENGTH: u16 = 32;
+const METADATA_DECIMALS: &str = "icrc1:decimals";
+const METADATA_NAME: &str = "icrc1:name";
+const METADATA_SYMBOL: &str = "icrc1:symbol";
+const METADATA_FEE: &str = "icrc1:fee";
+const METADATA_MAX_MEMO_LENGTH: &str = "icrc1:max_memo_length";
 
 #[cfg(not(feature = "u256-tokens"))]
 pub type Tokens = ic_icrc1_tokens_u64::U64;
@@ -612,6 +617,28 @@ fn default_decimals() -> u8 {
     ic_ledger_core::tokens::DECIMAL_PLACES as u8
 }
 
+fn map_metadata_or_trap(arg_metadata: Vec<(String, Value)>) -> Vec<(String, StoredValue)> {
+    const DISALLOWED_METADATA_FIELDS: [&str; 5] = [
+        METADATA_DECIMALS,
+        METADATA_NAME,
+        METADATA_SYMBOL,
+        METADATA_FEE,
+        METADATA_MAX_MEMO_LENGTH,
+    ];
+    arg_metadata
+        .into_iter()
+        .map(|(k, v)| {
+            if DISALLOWED_METADATA_FIELDS.contains(&k.as_str()) {
+                ic_cdk::trap(&format!(
+                    "Metadata field {} is reserved and cannot be set",
+                    k
+                ));
+            }
+            (k, StoredValue::from(v))
+        })
+        .collect()
+}
+
 impl Ledger {
     pub fn from_init_args(
         sink: impl Sink + Clone,
@@ -655,10 +682,7 @@ impl Ledger {
             token_symbol,
             token_name,
             decimals: decimals.unwrap_or_else(default_decimals),
-            metadata: metadata
-                .into_iter()
-                .map(|(k, v)| (k, StoredValue::from(v)))
-                .collect(),
+            metadata: map_metadata_or_trap(metadata),
             max_memo_length: max_memo_length.unwrap_or(DEFAULT_MAX_MEMO_LENGTH),
             feature_flags: feature_flags.unwrap_or_default(),
             maximum_number_of_accounts: 0,
@@ -840,12 +864,12 @@ impl Ledger {
             .into_iter()
             .map(|(k, v)| (k, StoredValue::into(v)))
             .collect();
-        records.push(Value::entry("icrc1:decimals", self.decimals() as u64));
-        records.push(Value::entry("icrc1:name", self.token_name()));
-        records.push(Value::entry("icrc1:symbol", self.token_symbol()));
-        records.push(Value::entry("icrc1:fee", Nat::from(self.transfer_fee())));
+        records.push(Value::entry(METADATA_DECIMALS, self.decimals() as u64));
+        records.push(Value::entry(METADATA_NAME, self.token_name()));
+        records.push(Value::entry(METADATA_SYMBOL, self.token_symbol()));
+        records.push(Value::entry(METADATA_FEE, Nat::from(self.transfer_fee())));
         records.push(Value::entry(
-            "icrc1:max_memo_length",
+            METADATA_MAX_MEMO_LENGTH,
             self.max_memo_length() as u64,
         ));
         records
@@ -857,10 +881,7 @@ impl Ledger {
 
     pub fn upgrade(&mut self, sink: impl Sink + Clone, args: UpgradeArgs) {
         if let Some(upgrade_metadata_args) = args.metadata {
-            self.metadata = upgrade_metadata_args
-                .into_iter()
-                .map(|(k, v)| (k, StoredValue::from(v)))
-                .collect();
+            self.metadata = map_metadata_or_trap(upgrade_metadata_args);
         }
         if let Some(token_name) = args.token_name {
             self.token_name = token_name;

rs/ledger_suite/icrc1/ledger/tests/tests.rs

@@ -248,6 +248,20 @@ fn encode_init_args_with_small_sized_archive(
     }
 }
 
+fn encode_init_args_with_provided_metadata(
+    args: ic_ledger_suite_state_machine_tests::InitArgs,
+) -> LedgerArgument {
+    match encode_init_args(args.clone()) {
+        LedgerArgument::Init(mut init_args) => {
+            init_args.metadata = args.metadata;
+            LedgerArgument::Init(init_args)
+        }
+        LedgerArgument::Upgrade(_) => {
+            panic!("BUG: Expected Init argument")
+        }
+    }
+}
+
 fn encode_upgrade_args() -> LedgerArgument {
     LedgerArgument::Upgrade(None)
 }
@@ -698,6 +712,22 @@ fn icrc1_test_upgrade_from_v1_not_possible() {
     );
 }
 
+#[test]
+fn test_setting_forbidden_metadata_in_init_works_in_v3_ledger() {
+    ic_ledger_suite_state_machine_tests::metadata::test_setting_forbidden_metadata_works_in_v3_ledger(
+        ledger_mainnet_v3_wasm(),
+        encode_init_args_with_provided_metadata,
+    );
+}
+
+#[test]
+fn test_setting_forbidden_metadata_not_possible() {
+    ic_ledger_suite_state_machine_tests::metadata::test_setting_forbidden_metadata_not_possible(
+        ledger_wasm(),
+        encode_init_args_with_provided_metadata,
+    );
+}
+
 mod metrics {
     use crate::{encode_init_args, encode_upgrade_args, ledger_wasm};
     use ic_ledger_suite_state_machine_tests::metrics::LedgerSuiteType;

rs/ledger_suite/tests/sm-tests/src/lib.rs

@@ -4786,3 +4786,152 @@ pub fn generate_transactions(
         start.elapsed()
     );
 }
+
+pub mod metadata {
+    use super::*;
+
+    const METADATA_DECIMALS: &str = "icrc1:decimals";
+    const METADATA_NAME: &str = "icrc1:name";
+    const METADATA_SYMBOL: &str = "icrc1:symbol";
+    const METADATA_FEE: &str = "icrc1:fee";
+    const METADATA_MAX_MEMO_LENGTH: &str = "icrc1:max_memo_length";
+    const FORBIDDEN_METADATA: [&str; 5] = [
+        METADATA_DECIMALS,
+        METADATA_NAME,
+        METADATA_SYMBOL,
+        METADATA_FEE,
+        METADATA_MAX_MEMO_LENGTH,
+    ];
+
+    pub fn test_setting_forbidden_metadata_works_in_v3_ledger<T>(
+        ledger_wasm_v3: Vec<u8>,
+        encode_init_args: fn(InitArgs) -> T,
+    ) where
+        T: CandidType,
+    {
+        let env = StateMachine::new();
+
+        let forbidden_metadata = vec![
+            Value::entry(METADATA_DECIMALS, 8u64),
+            Value::entry(METADATA_NAME, "BogusName"),
+            Value::entry(METADATA_SYMBOL, "BN"),
+            Value::entry(METADATA_FEE, Nat::from(10_000u64)),
+            Value::entry(METADATA_MAX_MEMO_LENGTH, 8u64),
+        ];
+
+        let args = encode_init_args(InitArgs {
+            metadata: forbidden_metadata.clone(),
+            ..init_args(vec![])
+        });
+        let args = Encode!(&args).unwrap();
+        let canister_id = env
+            .install_canister(ledger_wasm_v3.clone(), args, None)
+            .unwrap();
+
+        let verify_duplicate_metadata = || {
+            let metadata = Decode!(
+                &env.query(canister_id, "icrc1_metadata", Encode!().unwrap())
+                    .expect("failed to query metadata")
+                    .bytes(),
+                Vec<(String, Value)>
+            )
+            .expect("failed to decode metadata response");
+
+            let mut key_counts = HashMap::new();
+
+            for (k, _v) in metadata.iter() {
+                key_counts
+                    .entry(k.clone())
+                    .and_modify(|counter| *counter += 1)
+                    .or_insert(1);
+            }
+
+            // The forbidden metadata should be present twice - one instance from the init args, and
+            // one dynamically set by the ledger based on its internal state.
+            for forbidden_metadata in FORBIDDEN_METADATA.iter() {
+                assert_eq!(key_counts.get(*forbidden_metadata), Some(&2));
+            }
+        };
+
+        verify_duplicate_metadata();
+
+        let ledger_upgrade_arg = LedgerArgument::Upgrade(Some(UpgradeArgs {
+            metadata: Some(forbidden_metadata),
+            ..UpgradeArgs::default()
+        }));
+        env.upgrade_canister(
+            canister_id,
+            ledger_wasm_v3,
+            Encode!(&ledger_upgrade_arg).unwrap(),
+        )
+        .unwrap();
+
+        verify_duplicate_metadata();
+    }
+
+    pub fn test_setting_forbidden_metadata_not_possible<T>(
+        ledger_wasm: Vec<u8>,
+        encode_init_args: fn(InitArgs) -> T,
+    ) where
+        T: CandidType,
+    {
+        let env = StateMachine::new();
+
+        // Verify that specifying any of the forbidden metadata in the init args is not possible.
+        for forbidden_metadata in FORBIDDEN_METADATA.iter() {
+            let args = encode_init_args(InitArgs {
+                metadata: vec![Value::entry(*forbidden_metadata, 8u64)],
+                ..init_args(vec![])
+            });
+            let args = Encode!(&args).unwrap();
+            match env.install_canister(ledger_wasm.clone(), args, None) {
+                Ok(_) => {
+                    panic!("should not be able to install ledger with forbidden metadata")
+                }
+                Err(err) => {
+                    err.assert_contains(
+                        ErrorCode::CanisterCalledTrap,
+                        "is reserved and cannot be set",
+                    );
+                }
+            }
+        }
+
+        let args = encode_init_args(init_args(vec![]));
+        let args = Encode!(&args).unwrap();
+        let canister_id = env
+            .install_canister(ledger_wasm.clone(), args, None)
+            .expect("should successfully install ledger without forbidden metadata");
+
+        // Verify that also upgrading does not accept the forbidden metadata
+        for forbidden_metadata in FORBIDDEN_METADATA.iter() {
+            let ledger_upgrade_arg = LedgerArgument::Upgrade(Some(UpgradeArgs {
+                metadata: Some(vec![Value::entry(*forbidden_metadata, 8u64)]),
+                ..UpgradeArgs::default()
+            }));
+            match env.upgrade_canister(
+                canister_id,
+                ledger_wasm.clone(),
+                Encode!(&ledger_upgrade_arg).unwrap(),
+            ) {
+                Ok(_) => {
+                    panic!("should not be able to upgrade ledger with forbidden metadata")
+                }
+                Err(err) => {
+                    err.assert_contains(
+                        ErrorCode::CanisterCalledTrap,
+                        "is reserved and cannot be set",
+                    );
+                }
+            }
+        }
+
+        let ledger_upgrade_arg = LedgerArgument::Upgrade(Some(UpgradeArgs::default()));
+        env.upgrade_canister(
+            canister_id,
+            ledger_wasm.clone(),
+            Encode!(&ledger_upgrade_arg).unwrap(),
+        )
+        .expect("should successfully upgrade the ledger");
+    }
+}