-
Notifications
You must be signed in to change notification settings - Fork 220
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
feat(wallet): add grpc method for setting base node #3828
Conversation
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.
Thanks @zhangcheng looks good! You will just have to run cargo fmt --all
to pass clippy CI.
Regarding testing:
- manually: use a gRPC client to to call against your method with the wallet running
- automated: take a look at
integration_tests
folder (but you can leave this out for now)
Got it, thx @delta1 , will clippy it. |
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.
Looks good, will approve once Clippy passes.
I tried to use grpcurl to test against it, but my limited knowledge couldn't help me to construct valid inputs.
|
6016404
to
b1091c9
Compare
I haven't personally tested a CLI gRPC client at all, I prefer a GUI client like BloomRPC which can just ingest the proto file. from a glance that request looks ok except for the format of the |
I agree with @philipr-za it's much easier to test with a GUI. But not a showstopper here, if the CI passes then LGTM |
The address can be |
Just need to sign your commits |
Since I don't know how to construct a valid public key string for this case yet, the above string was pulled out of the log files, such as:
|
The grpc client often wants bytes to be base64, so you can try |
Something new for me to learn then. 😄 |
Cool, base64 string works. Thanks @mikethetike 🤝
Console output:
|
Well done 👏 |
applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs
Outdated
Show resolved
Hide resolved
Description --- Aim to resolve tari-project#3821 Motivation and Context --- Learning via "good first issue" How Has This Been Tested? --- Manual test with `grpcurl`
9fbe1e7
b7d40dc
to
9fbe1e7
Compare
Just want to say, BloomRPC is much nicer and easier to use than grpcurl, esp. to a newbie. @philipr-za |
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.
I preferred the binary version, but this is fine too.
I've queued it. If the CI passes, it will get merged. Thanks so much! |
* development: (28 commits) fix(wallet): fix aggressive disconnects in wallet connectivity (tari-project#3807) chore: honor decimals in ERC20 (tari-project#3809) chore: add icons to applications (tari-project#3812) fix: daily test (tari-project#3815) ci: improvements to macos pkg (tari-project#3824) ci: cargo test speedups (tari-project#3843) feat: add persistence of transaction cancellation reason to wallet db (tari-project#3842) fix: update RFC links and README (tari-project#3675) (tari-project#3839) chore: change NodeIdentity debug (tari-project#3817) fix(dan): include state_root in node hash (tari-project#3836) feat(cli): resize terminal height (tari-project#3838) feat(validator-node): initial state sync implementation (partial) (tari-project#3826) feat: update console wallet tui (tari-project#3837) feat: resize base node terminal on startup (tari-project#3827) feat(wallet): add grpc method for setting base node (tari-project#3828) chore(deps): bump follow-redirects from 1.14.5 to 1.14.8 in /applications/tari_web_extension_example (tari-project#3832) chore(deps): bump follow-redirects from 1.14.7 to 1.14.8 in /applications/tari_collectibles/web-app (tari-project#3833) chore(deps): bump follow-redirects from 1.14.5 to 1.14.8 in /applications/tari_web_extension (tari-project#3834) chore(deps): bump follow-redirects from 1.14.7 to 1.14.8 in /applications/launchpad/gui-vue (tari-project#3831) chore(deps): bump follow-redirects from 1.14.4 to 1.14.8 in /integration_tests (tari-project#3829) ...
Description
Aim to resolve #3821
Motivation and Context
Learning via "good first issue"
How Has This Been Tested?
Code compiles, and manual test with
grpcurl
.