Check for any existing wallet file in case wallet_file_name is given None.

[KnowWhoami]

Changing the wallet_name argument to be of type Option<String> in PR 291 introduces an internal bug.

Bug:

Now , taker/ makerd cli creates a new wallet each time we call there command if we do not pass the wallet_name argument.

Why:

coinswap/src/taker/api.rs

Lines 186 to 205 in 972585c

	let mut wallet = if let Some(file_name) = wallet_file_name {
	let wallet_path = wallets_dir.join(&file_name);
	rpc_config.wallet_name = file_name;
	if wallet_path.exists() {
	// Try loading wallet
	let wallet = Wallet::load(&rpc_config, &wallet_path)?;
	log::info!("Wallet file at {:?} successfully loaded.", wallet_path);
	wallet
	} else {
	// Create wallet with the given name.
	let mnemonic = Mnemonic::generate(12).unwrap();
	let seedphrase = mnemonic.to_string();
	let wallet = Wallet::init(&wallet_path, &rpc_config, seedphrase, "".to_string())?;
	log::info!("New Wallet created at : {:?}", wallet_path);
	wallet
	}
	} else {
	// Create default wallet
	let mnemonic = Mnemonic::generate(12).unwrap();

As per our current implemetation, we first check the value of wallet_file_path -> if it is None -> then it creates a new wallet.

Why we don't get this error before this change?

Before that , args.wallet_name is set to the default value i.e taker in case of taker-cli before passing it to Taker::init api -> that's why it often loads up the same wallet even though we do not pass the wallet argument.

This Pr aims to fix this bug by searching for any existing wallet file in the wallets_dir in case wallet_file_path is passed as None.
It also simplifies the wallet related logic in both Taker::init and Maker::init.

Solution (After discussion):

• take default wallet name as maker/taker-wallet if wallet_file_name=None instead of maker-unique_identifier where this identifier is created from mnemonics.

Note to Reviewers:

• In 2e4f8e4
  I try to make file_path input more flexible by considering impl<AsRef<Path>> instead of &PathBuf or &Path etc
  as mentioned in Rust Design pattern

• This pattern is often used in low level crates as it provides flexibility at the caller site that it can take any value of type which must implements AsRef<Path>.

• IMO, It would be useful when we will modularise our crate but for now, It seems a bit over-engineered to me.
  what's your say?

[codecov]

Codecov Report

Attention: Patch coverage is 74.35897% with 10 lines in your changes missing coverage. Please review.

Project coverage is 73.56%. Comparing base (41c43de) to head (3096859).
Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
src/wallet/api.rs	63.63%	4 Missing ⚠️
src/maker/api.rs	72.72%	3 Missing ⚠️
src/taker/api.rs	66.66%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #305      +/-   ##
==========================================
+ Coverage   73.51%   73.56%   +0.05%     
==========================================
  Files          33       33              
  Lines        4104     4082      -22     
==========================================
- Hits         3017     3003      -14     
+ Misses       1087     1079       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mojoX911]

Concept Ack.

But it needs some more thinking and redesign. Our random naming behavior is not at all UX friendly, and better if we drop it or keep it test only.

Deatils below.

[Shourya742]

ACK. Just minor nits. We'll remove the static dispatch stuff once we've finalized our APIs. Until then, there's no harm in keeping them, though they might slightly increase the binary size, but not by a significant margin.

[mojoX911]

Ack. Great cleanup. Just one nit. No other blocking comments from my side.

This now nicely goes into following fixes for UX that can be done in followup PRs.

• Show the mnemonic, only once at init, so the user can back it up. This will require bringing back bip39.

• Allow for wallet discovery using a mnemonic. User puts in the mnemonic and the wallet find all the seed and fidelity utxos. Swapcoins would be lost tho, for that we need to find some ways to backup the swap data.

• Add password protection on the wallet. Its embarrassing to have wallet data in clear. We have postponed it so far but now is the time to patch it. If we can use some encrypt decrypt API from secp or rust-bitcoin lib that would be best.

[KnowWhoami]

Ack. Great cleanup. Just one nit. No other blocking comments from my side.

This now nicely goes into following fixes for UX that can be done in followup PRs.
Show the mnemonic, only once at init, so the user can back it up. This will require bringing back bip39.

Including the first task here itself , as it's small change.
created an issue for rest tasks #333

[KnowWhoami]

It's ready.

[mojoX911]

Ack