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(forge): add calldata and returndata views to debugger #7110

Conversation

emo-eth
Copy link
Contributor

@emo-eth emo-eth commented Feb 13, 2024

Motivation

It's often useful to see what data is actually in calldata/returndata, as well as see which regions are being operated on.

Closes #5492.

Solution

  • Add calldata and returndata to DebugStep
  • Add active_buffer to DebuggerContext
  • get_memory_access -> get_buffer_accesses; note which buffer is being read from, if any; simplify return type
  • draw_memory -> draw_buffer; compare read/writes to current active buffer
  • add b ("buffer") operator to debugger to cycle Memory -> Calldata -> Returndata buffers in the "buffer" panel view
  • add b to help string; update references of "memory" to "buffer"

Unsure of where debugger tests live (if anywhere), but draw logic seems to handle edge cases well; I'd be most concerned about potential performance issues or highlighting regions in the wrong buffer.

Screenshots:

Calldata access:
Screenshot 2024-02-13 at 2 10 15 PM
Memory overwrite from calldatacopy:
Screenshot 2024-02-13 at 2 11 32 PM
Returndata access:
Screenshot 2024-02-13 at 2 10 42 PM
Memory overwrite from returndatacopy:
Screenshot 2024-02-13 at 2 10 55 PM

Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

Nice, I like this, just a nit

opcode::CREATE | opcode::CREATE2 => (2, 3, 0, 0),
opcode::CALL | opcode::CALLCODE => (4, 5, 0, 0),
opcode::DELEGATECALL | opcode::STATICCALL => (3, 4, 0, 0),
) -> (Option<BufferKind>, Option<usize>, Option<usize>, Option<usize>, Option<usize>) {
Copy link
Member

Choose a reason for hiding this comment

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

Clippy is complaining about this, which is fair. You can extract it into a struct and return one Option<_>

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.

sweet!
pending @DaniPopes

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

awesome work. Pending clippy & @DaniPopes

@DaniPopes DaniPopes merged commit e10b039 into foundry-rs:master Feb 14, 2024
19 checks passed
emo-eth added a commit to emo-eth/foundry that referenced this pull request Feb 14, 2024
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.

feat(forge): Forge debugger should have calldata and returndata buffer views
4 participants