feat(evm): record executions with an inspector - #375
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8f43952b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// Refusing whole records rather than trimming one keeps every record in the | ||
| /// stream complete, so a reader never meets a half-written one. | ||
| fn fits(&mut self, length: usize) -> bool { | ||
| if self.stream.len() + length <= self.options.limit as usize { |
There was a problem hiding this comment.
When a record does not fit, this only sets truncated and leaves the current stream length unchanged, so later smaller records can still be appended. With inputs such as a large call/create payload or large return data, the trace then contains holes rather than a prefix of the execution, and Inspector.tree can pair later end events with the wrong open frame. Once the limit is hit, fits should keep returning false for the rest of that execution.
Useful? React with 👍 / 👎.
| self.u16(interp.message().depth); | ||
| self.u64(interp.gas().remaining()); | ||
| self.u32(if self.options.memory { interp.memory().len() as u32 } else { 0 }); | ||
| self.u8(stack as u8); |
There was a problem hiding this comment.
When stack: true, valid EVM executions can have more than 255 stack items (the limit is 1024), but this casts the count to u8 while still writing every stack word. The TypeScript decoder then consumes only the wrapped count and interprets the remaining stack bytes as later event tags or an early END, so a successful transaction can return a corrupt or silently incomplete trace. Encode the count with a wider field or cap the number of words written to the encoded count.
Useful? React with 👍 / 👎.
| Ok(result) => { | ||
| let mut writer = Writer::new(); | ||
| write_result(&mut writer, &result); | ||
| write_trace(&mut writer); |
There was a problem hiding this comment.
For an async EVM, driver.until replays the transaction after a pending database read, but the collector is drained only on the successful path here. If an abandoned attempt recorded hooks before returning PENDING, those partial events remain in the collector and get prepended to the final retry's trace, producing duplicate or stale events; reset/drain the collector at the start of each attempt or when returning PENDING (the same pattern applies to transact).
Useful? React with 👍 / 👎.
| _interp: &mut Interpreter<'_, '_, BaseEvmTypes>, | ||
| message: &mut Message<BaseEvmTypes>, | ||
| ) -> Option<MessageResult<BaseEvmTypes>> { | ||
| if self.fits(1 + 71 + 4 + message.input.len()) { |
There was a problem hiding this comment.
This fit check budgets a call/create message as 76 + input bytes, but message() writes the tag plus kind/depth/gas, three addresses, a 32-byte value, the input length, and input (108 + input). With a user-supplied limit, many small calls can grow the trace well past the advertised byte cap before truncation is reported, defeating the bound that protects memory. Include the actual encoded length in both call and create checks.
Useful? React with 👍 / 👎.
| evm: Evm<boolean>, | ||
| options: Inspector.Options = {}, | ||
| ): void { | ||
| evm['~engine'].setInspector({ |
There was a problem hiding this comment.
For async databases, every operation that touches the engine is funneled through attempt/Driver.serialize, but this direct call bypasses that queue. If a callTx or transact promise is paused on a pending read, setInspector or clearInspector can mutate the collector before the operation replays, so the in-flight execution is traced with the new settings (or not traced) rather than the settings in effect when it was started; make these setters Awaitable and serialize them like setBlock.
Useful? React with 👍 / 👎.
| (frame.selfdestructs as Frame['selfdestructs'][number][]).push({ | ||
| target: event.target, | ||
| value: event.value, |
There was a problem hiding this comment.
For any SELFDESTRUCT, the raw event includes both contract (the account being destroyed) and target (the beneficiary), but the tree view drops contract and exposes only the beneficiary/value. Consumers using Inspector.tree therefore cannot tell which account was removed, and guessing from the frame is wrong for contexts such as delegated execution; include event.contract in the frame's selfdestruct entry.
Useful? React with 👍 / 👎.
1 participant
Adds
Evm.setInspectorandEvm.clearInspector, which record what an execution did and report it as a trace on the result, plusInspector.treeandInspector.stepsto read it. The adapter records the hooks itself rather than calling into JavaScript per opcode, since evm2'sInspectorresolves at compile time.Stacked on #366.