feat(node): make spend and cash_note reason field configurable #1650
Conversation
maqi
changed the title
maqi
force-pushed
the
spend_reason
branch
2 times, most recently
from
c39bee9 to
4db5135
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -105,11 +105,13 @@ | |||
| let root_dir = std::env::temp_dir(); | |||
| trace!("Starting Kad swarm in client mode..{root_dir:?}."); | |||
|
|
|||
| // TODO: shall client bearing owner's discord user name, to be reflected in the cash_notes? | |||
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note
maqi
force-pushed
the
spend_reason
branch
4 times, most recently
from
a3934b0 to
80db435
Compare
maqi
changed the title
| /// Specify the owner(readable discord user name). | ||
| #[clap(long)] | ||
| owner: Option<String>, |
There was a problem hiding this comment.
This is just for a local network and I assue we might need to pass in the onwer inside the Add arg to enable this for service nodes.
There was a problem hiding this comment.
as discussed, this can be addressed in follow up PR
| @@ -286,8 +296,31 @@ pub async fn run_node( | |||
| launcher: &dyn Launcher, | |||
| rpc_client: &dyn RpcActions, | |||
| ) -> Result<NodeServiceData> { | |||
| let user = match get_username() { | |||
There was a problem hiding this comment.
get_username() will at most cases return our OS's user account name. We can just use that. I'm not sure if we might ever have a failure here. Also since this is just for a local network, should we want to pass in the owner via CLI? It is just for testing right?
There was a problem hiding this comment.
I'm not sure if we might ever have a failure here
If we do have a failure, we can use a default value like "owner" since it is a local network.
There was a problem hiding this comment.
as discussed, this can be addressed in follow up PR
| /// For what purpose this cash_note was created | ||
| pub reason: String, |
There was a problem hiding this comment.
This can be removed on a followup PR as discussed.
| /// The reason this cash_note created for | ||
| /// eg. `store cost pay to...`, `network royalty`, `change to self`, `payment transfer`, ... | ||
| pub reason: String, |
There was a problem hiding this comment.
This can be removed on a followup PR as discussed.
Description
reviewpad:summary