-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(cheats): preserve struct order when parsing JSON objects #11069
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
base: master
Are you sure you want to change the base?
Conversation
…oundry into rusowsky/cheats-sorted-json
@klkvr / @DaniPopes when time permits, would you mind reviewing this? I think you two are the most familiar with the JSON cheatcode / Solar HIR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will be able to be implemented by passing along the solar context from the build output #11307
@@ -369,14 +369,15 @@ fn persist_caller(ccx: &mut CheatsCtxt) { | |||
} | |||
|
|||
/// Performs an Ethereum JSON-RPC request to the given endpoint. | |||
fn rpc_call(url: &str, method: &str, params: &str) -> Result { | |||
fn rpc_call(struct_defs: &StructDefinitions, url: &str, method: &str, params: &str) -> Result { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's pass along state
instead of struct_defs
to all necessary functions
@@ -863,7 +864,7 @@ mod tests { | |||
root: PathBuf::from(&env!("CARGO_MANIFEST_DIR")), | |||
..Default::default() | |||
}; | |||
Cheatcodes::new(Arc::new(config)) | |||
Cheatcodes::new(Arc::new(config), StructDefinitions::default()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's keep the new
signature to the bare minimum required and add builder-style fns for extra stuff
should i wait for yours to be merge so that we don't need to reimplement? |
blocked until we have the required implementation in solar |
Motivation
Historically,
vm.parseJson
has had a limitation where fields had to be sorted in alphabetical order. However, now that we are integrating solar everywhere, there is an opportunity to leverage it to deterministically figure out the struct fields and their order, and overcome this limitation.Solution
Introduce a new field
struct_defs
to thestruct Cheatcodes
, which gives it access to a hashmap that stores all struct definitions and their (sorted) fields (with their type):