feat(wallet): using correct network #1249
Conversation
Test Results (CI)610 tests +3 595 ✅ - 12 3h 16m 40s ⏱️ - 20m 47s For more details on these failures, see this check. Results for commit d88fcb6. ± Comparison against base commit 25921fa. ♻️ This comment has been updated with latest results. |
|
Think the integration tests need to be updated - looks like the validators are on Esmeralda and wallet is on localnet in cucumber. Localnet should be used for cucumbers. Also need to update the wallet swarm command to pass in the |
Okay will check the integration tests and the command passing, just forgot about that thanks! |
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
| @@ -122,7 +122,7 @@ pub async fn handle_create( | |||
| ); | |||
|
|
|||
| let max_fee = req.max_fee.unwrap_or(DEFAULT_FEE); | |||
| let transaction = Transaction::builder() | |||
| let transaction = transaction_builder(context) | |||
There was a problem hiding this comment.
Sorry, just in case, idiomatic Rust use of functions assumes the following syntax: module_name::function_name, so I think it will be more idiomatic to use types::transaction_builder(context)
| @@ -150,3 +151,8 @@ pub(super) fn invalid_params<T: Display>(field: &str, details: Option<T>) -> any | |||
| ) | |||
| .into() | |||
| } | |||
|
|
|||
| /// Returns a TransactionBuilder with the current network configured. | |||
| pub fn transaction_builder(context: &HandlerContext) -> TransactionBuilder { | |||
There was a problem hiding this comment.
In my humble opinion it would be nice to make the name more verbose to what it's really do and maybe associate the method with TransactionBuilder type. For example TransactionBuilder::with_context(context).
There was a problem hiding this comment.
This is just a convinence method for the JSON-RPC handlers so not related to the transaction builder.
Please make the visibility of the function pub(super) to prevent it being used outside of the handlers module.
| @@ -323,6 +323,7 @@ pub async fn handle_reveal_funds( | |||
|
|
|||
| // If the caller aborts the request early, this async block would be aborted at any await point. To avoid this, we | |||
| // spawn a task that will continue running. | |||
| let ctx = context.clone(); | |||
There was a problem hiding this comment.
Maybe instead of cloning the whole context we can instantiate builder here and move it into task's scope? Like let builder_for_task = TransactionBuilder::with_contex(context);?
There was a problem hiding this comment.
I suggest let builder = transaction_builder(context); outside the spawn to avoid the clone (not really fussed about it though 😉) .
applications/tari_validator_node/src/transaction_validators/error.rs
Outdated
Show resolved
Hide resolved
…twork-to-wallet # Conflicts: # applications/tari_dan_wallet_daemon/src/handlers/accounts.rs # applications/tari_dan_wallet_daemon/src/handlers/helpers.rs # applications/tari_dan_wallet_daemon/src/handlers/nfts.rs
| @@ -323,6 +323,7 @@ pub async fn handle_reveal_funds( | |||
|
|
|||
| // If the caller aborts the request early, this async block would be aborted at any await point. To avoid this, we | |||
| // spawn a task that will continue running. | |||
| let ctx = context.clone(); | |||
There was a problem hiding this comment.
I suggest let builder = transaction_builder(context); outside the spawn to avoid the clone (not really fussed about it though 😉) .
6ed3e38
Description
For now we use the correct network configured and also validating on the validator node side in mempool.
Motivation and Context
Closes #1241
How Has This Been Tested?
Start a fresh swarm locally and send any transaction through wallet daemon. It is expected to still work.
What process can a PR reviewer use to test or verify this change?
Breaking Changes