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

Revamp and Add More to Stylus Cache Manager CLI #78

Merged
merged 11 commits into from
Aug 28, 2024
Merged

Conversation

rauljordan
Copy link
Contributor

@rauljordan rauljordan commented Aug 15, 2024

Description

This PR addresses all recent feedback on cache manager commands for cargo stylus. It includes the following:

  • cargo stylus cache bid [ADDRESS] [AMOUNT] with documentation
  • cargo stylus cache suggest-bid [ADDRESS], which gives the required bid for an address of a Stylus contract
  • cargo stylus status --address=... which provides a status update on the cache, including information about whether or not a specified address is cached and other data from the cache manager
Screenshot 2024-08-15 at 17 27 18

This PR also uses alloy in the entire cache package instead of ethers-rs in an attempt to unify the cargo stylus repo to stick with alloy for Ethereum tooling

Copy link
Contributor

@gligneul gligneul left a comment

Choose a reason for hiding this comment

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

Looks good! Just minor comments.

Comment on lines +96 to +103
println!("");
let note = format!(
r#"NOTE: We recommend running cargo stylus cache bid 0 {} to cache your activated contract in ArbOS.
Cached contracts benefit from cheaper calls. To read more about the Stylus contract cache, see
https://docs.arbitrum.io/stylus/concepts/stylus-cache-manager"#,
hex::encode(contract_addr),
).debug_mint();
println!("{note}");
Copy link
Contributor

Choose a reason for hiding this comment

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

This notice format didn't work well. It is printing \n instead of actual new lines. Also, the address and bid arguments are inverted here.

"NOTE: We recommend running cargo stylus cache bid 0 611696c1cff8c63f4b3c5308300d355dc80c4b65 to cache your activated contract in ArbOS.\nCached contracts benefit from cheaper calls. To read more about the Stylus contract cache, see\nhttps://docs.arbitrum.io/stylus/concepts/stylus-cache-manager"

check/src/cache.rs Outdated Show resolved Hide resolved
}

fn format_gas(gas: u128) -> String {
let gas: u128 = gas.try_into().unwrap_or(u128::MAX);
Copy link
Contributor

Choose a reason for hiding this comment

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

The linter pointed out that this line could be removed, and it made sense. Maybe in the future we should block PRs from being merged if we get linter warnings.

check/src/wallet.rs Show resolved Hide resolved
Co-authored-by: Gabriel de Quadros Ligneul <[email protected]>
@rauljordan
Copy link
Contributor Author

Screenshot 2024-08-28 at 09 33 42 Fixed it up, thanks for the feedback!

@rauljordan rauljordan merged commit 9237e85 into main Aug 28, 2024
7 checks passed
@rauljordan rauljordan deleted the cache-updates branch August 28, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants