Skip to content
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

Mpool CLI Commands #1203

Merged
merged 46 commits into from
Oct 18, 2021
Merged

Mpool CLI Commands #1203

merged 46 commits into from
Oct 18, 2021

Conversation

connormullett
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • Adds MemPool commands stat and pending

Reference issue to close (if applicable)

Closes
#568

Other information and links

Note: To test mpool stat, a fully synced node is required or else it will fail.

@ec2
Copy link
Member

ec2 commented Aug 6, 2021

This shouldn't build. There are merge conflicts.

@cryptoquick
Copy link
Contributor

cryptoquick commented Aug 23, 2021

@connormullett Is this RFR now? be sure to merge main, btw

@connormullett
Copy link
Contributor Author

@connormullett Is this RFR now? be sure to merge main, btw

Yup! Thanks for the reminder

Copy link
Contributor

@q9f q9f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

./forest/src/cli/mpool_cmd.rs was missing header
./node/rpc-client/src/mpool_ops.rs was missing header

min_base_fee = current_tipset.blocks()[0].parent_base_fee().clone();
}

let wallet_response = wallet_list().await.map_err(handle_rpc_err).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you discuss the usage of .unwarp() yet? I would prefer if we could reduce usage or at least relace them with something like .expect("Connor is certain this cannot fail ;-)")...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, I haven't looked at this in a while so I don't remember which ones are infallible. Just going to handle the errors with graceful exits using cli_error_and_die

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I remembered something important. handle_rpc_error will exit and wrap an error from the rpc-api, such that when it is unwrapped it will be verbose as it is the error coming from the API wrapped in a JsonRpcError type

let filter: Vec<&Address> = addresses
.iter()
.filter(|&addr| addr == &message.message().from)
.collect();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can skip collecting the filter and check directly is_none() afaiu...

diff --git a/forest/src/cli/mpool_cmd.rs b/forest/src/cli/mpool_cmd.rs
index 2bbaaeda..2c4a4c69 100644
--- a/forest/src/cli/mpool_cmd.rs
+++ b/forest/src/cli/mpool_cmd.rs
@@ -106,12 +106,12 @@ impl MpoolCommands {
 
                     for message in &messages {
                         if !addresses.is_empty() {
-                            let filter: Vec<&Address> = addresses
+                            if addresses
                                 .iter()
                                 .filter(|&addr| addr == &message.message().from)
-                                .collect();
-
-                            if filter.is_empty() {
+                                .next()
+                                .is_none()
+                            {
                                 continue;
                             }
                         }

@connormullett connormullett merged commit a33328c into main Oct 18, 2021
@connormullett connormullett deleted the connor/mpool-cli branch October 18, 2021 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants