-
Notifications
You must be signed in to change notification settings - Fork 260
Wallet daemon #788
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
base: master
Are you sure you want to change the base?
Wallet daemon #788
Conversation
Impl broadcast method --------- Co-authored-by: Maxim <[email protected]>
* wallet deaemon: Implement broadcast_replacemant * Submit only the first transaction as replacement, the rest just as-is * Re-format
* feat: create unsigned transaction --------- Co-authored-by: 143672 <[email protected]>
# Conflicts: # clippy.toml
# Conflicts: # Cargo.lock # wallet/core/src/api/message.rs # wallet/pskt/Cargo.toml
* feat: sign and sign_transactions improvements * fix: review
.github/workflows/ci.yaml
Outdated
| source musl-toolchain/build.sh | ||
| # Build for musl | ||
| cargo --verbose build --bin kaspad --bin rothschild --bin kaspa-wallet --release --target x86_64-unknown-linux-musl | ||
| cargo --verbose build --bin kaspad --bin rothschild --bin kaspa-wallet-daemon --release --target x86_64-unknown-linux-musl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the introduced deamon needs to also be built, it should be a new binary, probably not replacing the kaspa-wallet (cli)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wallet/native binary just reruns kaspa-cli.. there's no point to have such wrapper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rusty-kaspa/wallet/native/src/main.rs
Line 5 in 2ccc9e4
| let result = kaspa_cli(TerminalOptions::new().with_prompt("$ "), None).await; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How should Kaspa users (ex: me as a user) run the CLI without the codebase providing an entry point? (and potentially a built binary: cli.exe)
I guess it would require me to compile locally after having re-added such an entry-point file, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kaspa-cli is a binary. You can either run or build it. I don't see any difference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, got it now, thanks.
So I think we should introduce kaspa-cli now, so that its build is being tested in ci, and its builds still are published as release assets, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that makes sense. I thought it was already there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General comment: kaspa-wallet-daemon should be consider an addition, not a replacement to the already existing kaspa-wallet (cli)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rusty-kaspa/wallet/native/src/main.rs
Line 5 in 2ccc9e4
| let result = kaspa_cli(TerminalOptions::new().with_prompt("$ "), None).await; |
the crate was deleteed by @KaffinPX since its existence doesnt make sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wallet/core/src/account/mod.rs
Outdated
| // update address manager with the last used index | ||
| if update_address_indexes { | ||
| receive_address_manager.set_index(last_receive_address_index)?; | ||
| change_address_manager.set_index(last_change_address_index)?; | ||
|
|
||
| let metadata = self.metadata()?.expect("derivation accounts must provide metadata"); | ||
| let store = self.wallet().store().as_account_store()?; | ||
| store.update_metadata(vec![metadata]).await?; | ||
| self.clone().scan(None, None).await?; | ||
| self.wallet().notify(Events::AccountUpdate { account_descriptor: self.descriptor()? }).await?; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be a conflict handling artifact, this block is duplicated with the one previously in the code (line 777)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wallet/daemon/src/args.rs
Outdated
| Command::new("kaspawalletd") | ||
| .about(format!("{} (kaspawalletd) v{}", env!("CARGO_PKG_DESCRIPTION"), version())) | ||
| .version(env!("CARGO_PKG_VERSION")) | ||
| .arg(Arg::new("password").long("password").short('p').value_name("password").help("Path of password file")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add .required(true), otherwise the program panic if no password provided (non informative output)
replace "Path of password file" with "Password" or "Wallet secret", in implementation it's not used to provide a password file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| .long("name") | ||
| .short('n') | ||
| .value_name("name") | ||
| .value_parser(clap::value_parser!(String)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is value parser on String useful here?
According to clap documentation, String is the default value parser
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are other cases where the same pattern is used. I don't see any issues with explicitly passed parser
| Arg::new("name") | ||
| .long("name") | ||
| .short('n') | ||
| .value_name("name") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| kaspa_core::log::init_logger(None, ""); | ||
| let args = Args::parse(); | ||
|
|
||
| let wallet = Arc::new(Wallet::try_new(Wallet::local_store()?, Some(Resolver::default()), None)?); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: Golang daemon allowed user to select a specific wallet file (by explicit path)
|
@biryukovmaxim Please add a description to this PR |
…let for backward compatibility
Done |

Implement Rust wallet daemon inspired by kaspawalletd
Summary
This PR introduces a Rust-based gRPC wallet daemon as a replacement for the existing Go (
kaspawalletd) implementation.The primary goal is to provide a fully Rust-native wallet daemon that integrates cleanly with the existing Rust wallet framework and build system.
At this stage, the implementation is feature-complete enough for testing and review.
What’s included
kaspa-wallet-daemonbinary implemented in Rustwallet/grpc/core,wallet/grpc/server)kaspa-wallet-corekaspa-wallet-daemonCurrent limitations
Due to limitations in the current Rust wallet framework, the following are not yet supported:
These are explicitly left out for now and will be addressed in follow-up work once the wallet framework supports them properly.
How to test
1. Run a Kaspa node (testnet)
2. Run the Rust wallet daemon
3. Integration tests (Go)
We validated the gRPC handlers using an external Go test suite:
https://github.com/childhoodisend/rusty-kaspa-wallet-test