fix: store wallets.json with backend data files (#4175)

Fixes https://dfinity.atlassian.net/browse/SDK-1918

Author: ericswanson-dfinity

CHANGELOG.md

@@ -63,6 +63,11 @@ Implement `dfx ledger allowance` subcommand that complies with the [ICRC-2](http
 - Add VetKD types and methods to management canister IDL
 - The VetKD test key id `Bls12_381_G2:dfx_test_key` is now enabled when starting `dfx` with `--replica`.
 
+### fix: dfx will no longer try to use a nonexistent wallet after changing backend settings without --clean
+
+After running `dfx start` with different options, dfx would try to use a wallet that was created
+on a previous run, which would fail. Now, dfx will create a new wallet if the settings have changed.
+
 ## Dependencies
 
 ### Motoko

e2e/tests-dfx/error_context.bash

@@ -20,33 +20,35 @@ teardown() {
 
   assert_command dfx identity get-wallet
 
-  echo "invalid json" >"$E2E_SHARED_LOCAL_NETWORK_DATA_DIRECTORY/wallets.json"
+  WALLETS_JSON="$(shared_wallets_json)"
+
+  echo "invalid json" >"$WALLETS_JSON"
 
   assert_command_fail dfx identity get-wallet
-  assert_match "failed to parse contents of .*/network/local/wallets.json as json"
+  assert_match "failed to parse contents of .*/network/local/[0-9a-f]+/wallets.json as json"
   assert_match "expected value at line 1 column 1"
 
   assert_command_fail dfx wallet upgrade
-  assert_match "failed to parse contents of .*/network/local/wallets.json as json"
+  assert_match "failed to parse contents of .*/network/local/[0-9a-f]+/wallets.json as json"
   assert_match "expected value at line 1 column 1"
 
-  echo '{ "identities": {} }' >"$E2E_SHARED_LOCAL_NETWORK_DATA_DIRECTORY/wallets.json"
+  echo '{ "identities": {} }' >"$WALLETS_JSON"
 
   # maybe you were sudo when you made it
-  chmod u=w,go= "$E2E_SHARED_LOCAL_NETWORK_DATA_DIRECTORY/wallets.json"
+  chmod u=w,go= "$WALLETS_JSON"
   assert_command_fail dfx identity get-wallet
-  assert_match "failed to read .*/network/local/wallets.json"
+  assert_match "failed to read .*/network/local/[0-9a-f]+/wallets.json"
   assert_match "Permission denied"
 
   assert_command_fail dfx wallet upgrade
-  assert_match "failed to read .*/network/local/wallets.json"
+  assert_match "failed to read .*/network/local/[0-9a-f]+/wallets.json"
   assert_match "Permission denied"
 
   # can't write it?
-  chmod u=r,go= "$E2E_SHARED_LOCAL_NETWORK_DATA_DIRECTORY/wallets.json"
+  chmod u=r,go= "$WALLETS_JSON"
   assert_command dfx identity new --storage-mode plaintext alice
   assert_command_fail dfx identity get-wallet --identity alice
-  assert_match "failed to write to .*/local/wallets.json"
+  assert_match "failed to write to .*/local/[0-9a-f]+/wallets.json"
   assert_match "Permission denied"
 }
 

e2e/tests-dfx/network.bash

@@ -30,17 +30,17 @@ teardown() {
 }
 
 @test "create with wallet stores canister ids for configured-ephemeral networks in canister_ids.json" {
+  echo "{}" | jq '.local.type="ephemeral"' >"$E2E_NETWORKS_JSON"
+
   dfx_start
 
-  setup_actuallylocal_shared_network
-  jq '.actuallylocal.type="ephemeral"' "$E2E_NETWORKS_JSON" | sponge "$E2E_NETWORKS_JSON"
-  dfx_set_wallet
+  dfx_set_wallet local
 
-  dfx canister create --all --network actuallylocal
+  dfx canister create --all
 
   # canister creates writes to a spinner (stderr), not stdout
-  assert_command dfx canister id e2e_project_backend --network actuallylocal
-  assert_match "$(jq -r .e2e_project_backend.actuallylocal .dfx/actuallylocal/canister_ids.json)"
+  assert_command dfx canister id e2e_project_backend
+  assert_match "$(jq -r .e2e_project_backend.local .dfx/local/canister_ids.json)"
 }
 
 @test "create stores canister ids for default-ephemeral local networks in .dfx/{network}canister_ids.json" {

e2e/tests-dfx/project_local_network.bash

@@ -108,11 +108,13 @@ teardown() {
 @test "with a shared network, wallet id is stored in the shared location" {
   dfx_start
 
-  assert_file_not_exists "$E2E_SHARED_LOCAL_NETWORK_DATA_DIRECTORY/wallets.json"
+  WALLETS_JSON="$(shared_wallets_json)"
+
+  assert_file_not_exists "$WALLETS_JSON"
 
   WALLET_ID="$(dfx identity get-wallet)"
 
-  assert_command jq -r .identities.default.local "$E2E_SHARED_LOCAL_NETWORK_DATA_DIRECTORY/wallets.json"
+  assert_command jq -r .identities.default.local "$WALLETS_JSON"
   assert_eq "$WALLET_ID"
   assert_file_not_exists .dfx/local/wallets.json
 }

e2e/tests-dfx/wallet.bash

@@ -13,15 +13,15 @@ teardown() {
 }
 
 @test "error reporting for --wallet parameter" {
+  dfx_start
   assert_command_fail dfx canister call hello_backend greet '' --with-cycles 100 --wallet "abc-def"
   assert_contains "Failed to read principal from id 'abc-def', and did not find a wallet for that identity"
   assert_contains "Text must be in valid Base32 encoding"
 
   assert_command_fail dfx canister call hello_backend greet '' --with-cycles 100 --wallet "alice"
   assert_contains "Failed to read principal from id 'alice', and did not find a wallet for that identity"
 
-  mkdir -p "$E2E_SHARED_LOCAL_NETWORK_DATA_DIRECTORY"
-  echo "{}" > "$E2E_SHARED_LOCAL_NETWORK_DATA_DIRECTORY/wallets.json"
+  echo "{}" > "$(shared_wallets_json)"
   assert_command_fail dfx canister call hello_backend greet '' --with-cycles 100 --wallet broken
   assert_contains "Failed to read principal from id 'broken' (Text must be in valid Base32 encoding.), and failed to load the wallet for that identity"
   assert_contains "missing field \`identities\`"
@@ -236,11 +236,41 @@ teardown() {
 }
 
 @test "detects if there is no wallet to upgrade" {
+  dfx_start
   dfx_new hello
   assert_command_fail dfx wallet upgrade
   assert_match "There is no wallet defined for identity 'default' on network 'local'.  Nothing to do."
 }
 
+@test "creates a new wallet when switching between pocketic and replica" {
+  dfx_new hello
+
+  USE_REPLICA=1 dfx_start --artificial-delay 101
+  dfx deploy
+
+  dfx_stop
+
+  USE_REPLICA="" dfx_start --artificial-delay 99
+  dfx deploy
+}
+
+@test "creates new wallet if backend changes" {
+  dfx_new hello
+
+  dfx_start --artificial-delay 101
+  dfx deploy
+
+  dfx_stop
+
+  dfx_start --artificial-delay 99
+  dfx deploy
+}
+
+@test "must run dfx start before accessing wallet on shared local network" {
+    assert_command_fail dfx wallet upgrade
+    assert_contains "cannot use a wallet before dfx start"
+}
+
 @test "redeem-faucet-coupon can set a new wallet and top up an existing one" {
   dfx_new hello
   dfx_start
@@ -254,7 +284,7 @@ teardown() {
   # prepare wallet to hand out
   dfx wallet balance # this creates a new wallet with user faucet_testing as controller
   dfx canister call faucet set_wallet_to_hand_out "(principal \"$(dfx identity get-wallet)\")" # register the wallet as the wallet that the faucet will return
-  rm "$E2E_SHARED_LOCAL_NETWORK_DATA_DIRECTORY/wallets.json" # forget about the currently configured wallet
+  find "$E2E_SHARED_LOCAL_NETWORK_DATA_DIRECTORY" -name wallets.json -exec rm {} \; # forget about the currently configured wallet
 
   # assert: no wallet configured
   export DFX_DISABLE_AUTO_WALLET=1

e2e/utils/_.bash

@@ -220,12 +220,20 @@ dfx_stop() {
 }
 
 dfx_set_wallet() {
+  local network="${1:-actuallylocal}"
+
   export WALLET_CANISTER_ID
   WALLET_CANISTER_ID=$(dfx identity get-wallet)
-  assert_command dfx identity set-wallet "${WALLET_CANISTER_ID}" --force --network actuallylocal
+  assert_command dfx identity set-wallet "${WALLET_CANISTER_ID}" --force --network "${network}"
   assert_match 'Wallet set successfully.'
 }
 
+shared_wallets_json() {
+  SETTINGS_DIGEST="$(jq -r .settings_digest "$E2E_SHARED_LOCAL_NETWORK_DATA_DIRECTORY/network-id")"
+  WALLETS_JSON="$E2E_SHARED_LOCAL_NETWORK_DATA_DIRECTORY/$SETTINGS_DIGEST/wallets.json"
+  echo "$WALLETS_JSON"
+}
+
 setup_actuallylocal_project_network() {
     webserver_port=$(get_webserver_port)
     # [ ! -f "$E2E_ROUTE_NETWORKS_JSON" ] && echo "{}" >"$E2E_ROUTE_NETWORKS_JSON"

src/dfx-core/src/config/directories.rs

@@ -1,12 +1,15 @@
-use crate::error::config::ConfigError;
+use crate::config::model::local_server_descriptor::NetworkMetadata;
 use crate::error::config::ConfigError::{
     DetermineConfigDirectoryFailed, EnsureConfigDirectoryExistsFailed,
 };
+use crate::error::config::{ConfigError, GetSharedWalletConfigPathError};
 use crate::error::get_user_home::GetUserHomeError;
 use crate::error::get_user_home::GetUserHomeError::NoHomeInEnvironment;
 #[cfg(not(windows))]
 use crate::foundation::get_user_home;
 use crate::fs::composite::ensure_dir_exists;
+use crate::identity::WALLET_CONFIG_FILENAME;
+use crate::json::load_json_file;
 use directories_next::ProjectDirs;
 use std::path::PathBuf;
 
@@ -24,6 +27,23 @@ pub fn get_shared_network_data_directory(network: &str) -> Result<PathBuf, GetUs
         .join(network))
 }
 
+pub fn get_shared_wallet_config_path(
+    network: &str,
+) -> Result<Option<PathBuf>, GetSharedWalletConfigPathError> {
+    let data_dir = get_shared_network_data_directory(network)?;
+
+    let network_id_path = data_dir.join("network-id");
+    if network_id_path.exists() {
+        let network_metadata: NetworkMetadata = load_json_file(&network_id_path)?;
+        let path = data_dir
+            .join(network_metadata.settings_digest)
+            .join(WALLET_CONFIG_FILENAME);
+        Ok(Some(path))
+    } else {
+        Ok(None)
+    }
+}
+
 pub fn get_user_dfx_config_dir() -> Result<PathBuf, ConfigError> {
     let config_root = std::env::var_os("DFX_CONFIG_ROOT");
     // dirs-next is not used for *nix to preserve existing paths

src/dfx-core/src/config/model/network_descriptor.rs

@@ -7,7 +7,7 @@ use crate::error::network_config::NetworkConfigError::{NetworkHasNoProviders, Ne
 use crate::error::uri::UriError;
 use candid::Principal;
 use slog::Logger;
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
 use url::Url;
 
 pub const MAINNET_MOTOKO_PLAYGROUND_CANISTER_ID: Principal =
@@ -18,7 +18,7 @@ pub const MOTOKO_PLAYGROUND_CANISTER_TIMEOUT_SECONDS: u64 = 1200;
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum NetworkTypeDescriptor {
     Ephemeral {
-        wallet_config_path: PathBuf,
+        wallet_config_path: Option<PathBuf>,
     },
     Playground {
         playground_canister: Principal,
@@ -40,7 +40,7 @@ pub struct NetworkDescriptor {
 impl NetworkTypeDescriptor {
     pub fn new(
         r#type: NetworkType,
-        ephemeral_wallet_config_path: &Path,
+        ephemeral_wallet_config_path: Option<PathBuf>,
         playground: Option<PlaygroundConfig>,
     ) -> Result<Self, NetworkConfigError> {
         if let Some(playground_config) = playground {
@@ -57,7 +57,7 @@ impl NetworkTypeDescriptor {
         } else {
             match r#type {
                 NetworkType::Ephemeral => Ok(NetworkTypeDescriptor::Ephemeral {
-                    wallet_config_path: ephemeral_wallet_config_path.to_path_buf(),
+                    wallet_config_path: ephemeral_wallet_config_path,
                 }),
                 NetworkType::Persistent => Ok(NetworkTypeDescriptor::Persistent),
             }

src/dfx-core/src/error/config.rs

@@ -3,6 +3,7 @@ use crate::error::fs::{
     CanonicalizePathError, CreateDirAllError, EnsureDirExistsError, NoParentPathError,
 };
 use crate::error::get_user_home::GetUserHomeError;
+use crate::error::structured_file::StructuredFileError;
 use handlebars::RenderError;
 use std::path::PathBuf;
 use thiserror::Error;
@@ -113,3 +114,12 @@ pub enum MergeTechStackError {
     #[error("expected extension canister type tech_stack to be an object")]
     ExpectedExtensionCanisterTypeTechStackObject,
 }
+
+#[derive(Error, Debug)]
+pub enum GetSharedWalletConfigPathError {
+    #[error(transparent)]
+    GetUserHome(#[from] GetUserHomeError),
+
+    #[error(transparent)]
+    StructuredFileError(#[from] StructuredFileError),
+}

src/dfx-core/src/error/identity.rs

@@ -1,3 +1,4 @@
+use crate::error::config::GetSharedWalletConfigPathError;
 use crate::error::fs::{
     ReadFileError, ReadPermissionsError, RemoveDirectoryAndContentsError, RemoveDirectoryError,
     RemoveFileError, RenameError, SetPermissionsError, WriteFileError,
@@ -209,7 +210,10 @@ pub enum MapWalletsToRenamedIdentityError {
     GetConfigDirectoryFailed(#[source] ConfigError),
 
     #[error(transparent)]
-    GetSharedNetworkDataDirectoryFailed(#[from] GetUserHomeError),
+    GetSharedNetworkDataDirectoryFailed(#[from] GetSharedWalletConfigPathError),
+
+    #[error(transparent)]
+    LoadNetworkMetadata(#[from] StructuredFileError),
 
     #[error("Failed to rename wallet global config key")]
     RenameWalletGlobalConfigKeyFailed(#[source] RenameWalletGlobalConfigKeyError),

src/dfx-core/src/error/network_config.rs

@@ -1,7 +1,8 @@
-use crate::error::config::{ConfigError, GetTempPathError};
+use crate::error::config::{ConfigError, GetSharedWalletConfigPathError, GetTempPathError};
 use crate::error::fs::ReadToStringError;
 use crate::error::get_user_home::GetUserHomeError;
 use crate::error::socket_addr_conversion::SocketAddrConversionError;
+use crate::error::structured_file::StructuredFileError;
 use crate::error::uri::UriError;
 use candid::types::principal::PrincipalError;
 use std::num::ParseIntError;
@@ -28,9 +29,15 @@ pub enum NetworkConfigError {
         source: UriError,
     },
 
+    #[error(transparent)]
+    GetSharedWalletConfigPathError(#[from] GetSharedWalletConfigPathError),
+
     #[error(transparent)]
     GetTempPath(#[from] GetTempPathError),
 
+    #[error(transparent)]
+    LoadNetworkId(StructuredFileError),
+
     #[error("Network '{0}' does not specify any network providers.")]
     NetworkHasNoProviders(String),
 

src/dfx-core/src/error/wallet_config.rs

@@ -16,6 +16,9 @@ pub enum WalletConfigError {
 
     #[error("Failed to save wallet configuration")]
     SaveWalletConfig(#[from] SaveWalletConfigError),
+
+    #[error("cannot use a wallet before dfx start")]
+    NoWalletBeforeDfxStart,
 }
 
 #[derive(Error, Debug)]

src/dfx-core/src/identity/mod.rs

@@ -2,7 +2,7 @@
 //!
 //! Wallets are a map of network-identity, but don't have their own types or manager
 //! type.
-use crate::config::directories::{get_shared_network_data_directory, get_user_dfx_config_dir};
+use crate::config::directories::{get_shared_wallet_config_path, get_user_dfx_config_dir};
 use crate::config::model::network_descriptor::NetworkDescriptor;
 use crate::error::wallet_config::SaveWalletConfigError;
 use crate::error::{
@@ -256,16 +256,15 @@ impl Identity {
             )
             .map_err(RenameWalletGlobalConfigKeyFailed)?;
         }
-        let shared_local_network_wallet_path = get_shared_network_data_directory("local")
-            .map_err(MapWalletsToRenamedIdentityError::GetSharedNetworkDataDirectoryFailed)?
-            .join(WALLET_CONFIG_FILENAME);
-        if shared_local_network_wallet_path.exists() {
-            Identity::rename_wallet_global_config_key(
-                original_identity,
-                renamed_identity,
-                shared_local_network_wallet_path,
-            )
-            .map_err(RenameWalletGlobalConfigKeyFailed)?;
+        if let Some(shared_local_wallet_path) = get_shared_wallet_config_path("local")? {
+            if shared_local_wallet_path.exists() {
+                Identity::rename_wallet_global_config_key(
+                    original_identity,
+                    renamed_identity,
+                    shared_local_wallet_path,
+                )
+                .map_err(RenameWalletGlobalConfigKeyFailed)?;
+            }
         }
         if let Some(temp_dir) = project_temp_dir {
             let local_wallet_path = temp_dir.join("local").join(WALLET_CONFIG_FILENAME);

src/dfx-core/src/identity/wallet.rs

@@ -25,7 +25,9 @@ pub fn get_wallet_config_path(
                 .join(name)
                 .join(WALLET_CONFIG_FILENAME)
         }
-        NetworkTypeDescriptor::Ephemeral { wallet_config_path } => wallet_config_path.clone(),
+        NetworkTypeDescriptor::Ephemeral { wallet_config_path } => wallet_config_path
+            .clone()
+            .ok_or(WalletConfigError::NoWalletBeforeDfxStart)?,
     })
 }