Skip to content

WIP feat: Seth command parity pt. 1#14

Merged
gakonst merged 17 commits intofoundry-rs:masterfrom
Anish-Agnihotri:aa/seth-commands
Sep 21, 2021
Merged

WIP feat: Seth command parity pt. 1#14
gakonst merged 17 commits intofoundry-rs:masterfrom
Anish-Agnihotri:aa/seth-commands

Conversation

@Anish-Agnihotri
Copy link
Copy Markdown
Contributor

@Anish-Agnihotri Anish-Agnihotri commented Sep 20, 2021

Adds more Seth commands:

@Anish-Agnihotri
Copy link
Copy Markdown
Contributor Author

Looks like there isn't a way to get localized timezone in Rust?

seth age is supposed to output Sun Sep 19 20:52:05 EDT 2021 (localized to timezone). Instead, returns Mon Sep 20 00:52:05 2021 (UTC).

@Anish-Agnihotri Anish-Agnihotri changed the title WIP feat: Seth command parity WIP feat: Seth command parity pt. 1 Sep 20, 2021
Copy link
Copy Markdown
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.

Great work. Some nits.

Comment thread dapptools/src/seth.rs Outdated
Comment on lines +41 to +42
unwrap_or_stdin(decimals)?,
unwrap_or_stdin(value)?
Copy link
Copy Markdown
Member

@gakonst gakonst Sep 20, 2021

Choose a reason for hiding this comment

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

Double stdin args don't work I believe, so best to make the first argument the stdin one, to be consistent with seth.

Comment thread dapptools/src/seth_opts.rs Outdated
ToDec { hexvalue: String },
#[structopt(name = "--to-fix")]
#[structopt(about = "convert integers into fixed point with specified decimals")]
ToFix { decimals: Option<u128>, value: Option<u128> },
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah I tested OG seth and you can only do echo 1 | seth --to-fix 3 (outputs 0.001), double stdin doesn't work so only have value being Option and have decimals be mandatory

Comment thread seth/src/lib.rs
pub fn namehash(ens: &str) -> Result<String> {
let mut node = vec![0u8; 32];

if !ens.is_empty() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's use this upstream function instead of re-implementing it.

Comment thread seth/src/lib.rs
/// Ok(())
/// }
/// ```
pub fn to_wei(value: u128, unit: String) -> Result<String> {
Copy link
Copy Markdown
Member

@gakonst gakonst Sep 20, 2021

Choose a reason for hiding this comment

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

Can you also add a command called format_units and parse_units which calls back to ethers::utils::{parse_units, format_units}? Maybe for the next PR.

Comment thread seth/src/lib.rs
Comment on lines +323 to +326
let mut ascii = String::new();
for letter in iter.collect::<Vec<_>>() {
ascii.push(letter.unwrap() as char);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is an example of something you'd do with fold

@gakonst gakonst mentioned this pull request Sep 21, 2021
@odyslam
Copy link
Copy Markdown
Contributor

odyslam commented Sep 21, 2021

For gas-price, I think it would be useful to return both the value from the provider and the one from gasnow API. What do you think @gakonst ?

Bonus points for some coloring in the return values. I made a similar PR in daptools OG and would be happy to port it here.

The end game is to use this command + seth estimate to get a view of what a contract deployment will cost you. It could be nice with automation, where you put the smart contract on hold to be automatically deployed at the gas price that you want.

@gakonst gakonst merged commit c2a55ba into foundry-rs:master Sep 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