Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up wallet config #811

Closed
wants to merge 2 commits into from
Closed

Clean up wallet config #811

wants to merge 2 commits into from

Conversation

666lcz
Copy link
Contributor

@666lcz 666lcz commented Mar 12, 2022

Not sure why we want to duplicate these code in two places. One thing that I would love to confirm with @patrickkuo or @arun-koshy is that why there's wallet.key and wallet.ks. Currently, the CLI tool will have wallet.key

wallet_config.keystore = KeystoreType::File(working_dir.join("wallet.key"));

while the REST server has wallet.ks

WalletConfig::create(&working_dir.join(wallet_path)).map_err(|error| {

Is it intentional that we want to use separate files? If yes, then we should add a comment in sui_command.rs and a new parameter wallet_key_path to WalletConfig::create

sui/sui/src/config.rs

Lines 115 to 129 in c3f7c73

impl Config for WalletConfig {
fn create(path: &Path) -> Result<Self, anyhow::Error> {
let working_dir = path
.parent()
.ok_or_else(|| anyhow!("Cannot determine parent directory."))?;
Ok(WalletConfig {
accounts: Vec::new(),
keystore: KeystoreType::File(working_dir.join("wallet.ks")),
gateway: GatewayType::Embedded(EmbeddedGatewayConfig {
db_folder_path: working_dir.join("client_db"),
..Default::default()
}),
config_path: path.to_path_buf(),
})
}

instead of overwriting the field

@666lcz 666lcz requested review from patrickkuo and arun-koshy March 12, 2022 20:41
@666lcz 666lcz force-pushed the chris/refactor-genesis branch from dd6c317 to 86e3e89 Compare March 12, 2022 20:52
@666lcz 666lcz force-pushed the chris/wallet-config branch from 3dceea9 to 18ed1b3 Compare March 12, 2022 20:52
@666lcz 666lcz changed the title Refactor Genesis Code Clean up wallet config Mar 12, 2022
@patrickkuo
Copy link
Contributor

I am working on refactoring the CLI and the rest server, starting with the configs files and keystore, I am trying to decouple rest server from wallet. The rest server is using the config files and keystore but it doesn't really need them persisted,

Can we hold this off for now?

@arun-koshy
Copy link
Contributor

There is a plan in place to refactor the rest server into a rest server + rest client/wallet application.

@666lcz 666lcz force-pushed the chris/refactor-genesis branch 2 times, most recently from 8b454f8 to b36aedb Compare March 16, 2022 18:17
@666lcz 666lcz changed the base branch from chris/refactor-genesis to main March 16, 2022 19:11
@666lcz 666lcz closed this Mar 16, 2022
@666lcz
Copy link
Contributor Author

666lcz commented Mar 16, 2022

No longer needed after #817

@huitseeker huitseeker deleted the chris/wallet-config branch May 27, 2022 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants