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

feat(cast): add tx command #260

Merged
merged 1 commit into from
Dec 19, 2021
Merged

feat(cast): add tx command #260

merged 1 commit into from
Dec 19, 2021

Conversation

tynes
Copy link
Contributor

@tynes tynes commented Dec 19, 2021

This commit ports the seth tx command to cast.
It follows the implementation of cast block and includes
the feature to print a specific field in the transaction
as well as printing the result in JSON.

$ cast tx --rpc-url <url> <tx-hash> [field]

The flag -j/--json is used to print the result
as JSON.

A whitespace is in a docstring is removed (autoformatted on save) and
the cast README is updated to indicate that the tx command was
implemented. Also check off the code command as it was previously
implemented.

Open to any stylistic feedback as I'm gaining experience writing Rust :)
The added doctest passes when I run it locally.

This commit ports the `seth tx` command to `cast`.
It follows the implementation of `cast block` and includes
the feature to print a specific field in the transaction
as well as printing the result in JSON.

```
$ cast tx --rpc-url <url> <tx-hash> [field]
```

The flag `-j`/`--json` is used to print the result
as JSON.
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

Comment on lines +340 to +341
.get(field)
.cloned()
Copy link
Member

Choose a reason for hiding this comment

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

there should be Value::take(field) which removes the value, so we could get rid of the clone here

Copy link
Member

Choose a reason for hiding this comment

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

eh in this case i guess the clone is not that expensive, one-time call expected anyway

Copy link
Member

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

LGTM - lints unrelated to the pr

Comment on lines +340 to +341
.get(field)
.cloned()
Copy link
Member

Choose a reason for hiding this comment

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

eh in this case i guess the clone is not that expensive, one-time call expected anyway

@gakonst gakonst merged commit 5679a42 into foundry-rs:master Dec 19, 2021
@tynes tynes deleted the feat/cast-tx branch December 19, 2021 06:44
@gakonst gakonst mentioned this pull request Dec 21, 2021
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.

3 participants