Conversation
WalkthroughThe PR updates performance metrics in the gas snapshot file and adds EVM_OP_TLOAD to the set of disallowed opcodes for the interpreter, restricting execution of this opcode alongside other storage-related restrictions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/interface/IExtrospectInterpreterV1.sol (1)
38-55: 🧹 Nitpick | 🔵 TrivialConsider whether
TSTOREshould also be explicitly listed inINTERPRETER_DISALLOWED_OPSfor clarity.
TSTOREreachesINTERPRETER_DISALLOWED_OPSindirectly throughNON_STATIC_OPS. The existingEVM_OP_CALLis redundantly listed in both sets with an explicit comment (Line 53: "Redundant with static list for clarity"). Given that the TLOAD comment specifically references TSTORE ("Interpreter cannot tstore so it has no reason to tload"), a reader might expect to see TSTORE explicitly listed here too for symmetry with the SSTORE/SLOAD pair.This is purely a readability suggestion — the bitmask is functionally correct as-is.
🤖 Fix all issues with AI agents
In @.gas-snapshot:
- Around line 1-63: Add missing tests for transient storage opcodes by creating
two new test functions (e.g., testTLoadooor and testTStoreooor) in the
ExtrospectionInterpreterTest suite mirroring the existing testSLoadooor and
testSStoreooor tests; these should execute bytecode that performs EVM_OP_TLOAD
and EVM_OP_TSTORE respectively and assert the interpreter disallows them
(revert/failure) per the disallowed list in IExtrospectInterpreterV1.sol. Place
the tests alongside other opcode tests in ExtrospectionInterpreterTest, use the
same structure/assertions as testSLoadooor/testSStoreooor so they show up in the
gas snapshot, and reference the disallowed-opcode behavior defined in
IExtrospectInterpreterV1.sol. Ensure test names exactly match the pattern
(testTLoadooor, testTStoreooor) so CI/gas snapshot picks them up.
| ExtrospectionBytecodeTest:testBytecode(address) (runs: 2048, μ: 330267, ~: 330289) | ||
| ExtrospectionBytecodeTest:testBytecodeHash(address) (runs: 2048, μ: 324944, ~: 324973) | ||
| ExtrospectionBytecodeTest:testScanEVMOpcodesPresentInAccount(address) (runs: 2048, μ: 325394, ~: 324878) | ||
| ExtrospectionBytecodeTest:testScanEVMOpcodesReachableInAccount(address) (runs: 2048, μ: 325651, ~: 324917) | ||
| ExtrospectionERC1167ProxyTest:testExtrospectionERC1167ProxyFailure(address,bytes) (runs: 2048, μ: 329785, ~: 329731) | ||
| ExtrospectionERC1167ProxyTest:testExtrospectionERC1167ProxySuccess(address,address) (runs: 2048, μ: 326706, ~: 326706) | ||
| ExtrospectionInterpreterTest:testCallCodeooor() (gas: 374857) | ||
| ExtrospectionInterpreterTest:testCallooor() (gas: 374856) | ||
| ExtrospectionInterpreterTest:testCreate2ooor() (gas: 373767) | ||
| ExtrospectionInterpreterTest:testCreateooor() (gas: 373439) | ||
| ExtrospectionInterpreterTest:testDelegateCallooor() (gas: 374508) | ||
| ExtrospectionInterpreterTest:testLog0ooor() (gas: 372675) | ||
| ExtrospectionInterpreterTest:testLog1ooor() (gas: 373047) | ||
| ExtrospectionInterpreterTest:testLog2ooor() (gas: 373397) | ||
| ExtrospectionInterpreterTest:testLog3ooor() (gas: 373809) | ||
| ExtrospectionInterpreterTest:testLog4ooor() (gas: 374116) | ||
| ExtrospectionInterpreterTest:testNoopooor() (gas: 356800) | ||
| ExtrospectionInterpreterTest:testSLoadooor() (gas: 375029) | ||
| ExtrospectionInterpreterTest:testSStoreooor() (gas: 371268) | ||
| ExtrospectionInterpreterTest:testSelfDestructooor() (gas: 370568) | ||
| LibExtrospectBytecodeCheckCBORTrimmedBytecodeHashTest:testCheckCBORTrimmedBytecodeHashFailure() (gas: 9093) | ||
| LibExtrospectBytecodeCheckCBORTrimmedBytecodeHashTest:testCheckCBORTrimmedBytecodeHashMetadataNotTrimmed() (gas: 12708) | ||
| LibExtrospectBytecodeCheckCBORTrimmedBytecodeHashTest:testCheckCBORTrimmedBytecodeHashSuccess() (gas: 7702) | ||
| LibExtrospectBytecodeIsEOFBytecodeTest:testCheckNotEOFBytecodeDoesNotRevertOnNonEOF() (gas: 1296) | ||
| LibExtrospectBytecodeIsEOFBytecodeTest:testCheckNotEOFBytecodeRevertsOnEOF() (gas: 4157) | ||
| LibExtrospectBytecodeIsEOFBytecodeTest:testIsEOFBytecodeEOF() (gas: 384) | ||
| LibExtrospectBytecodeIsEOFBytecodeTest:testIsEOFBytecodeEmpty() (gas: 366) | ||
| LibExtrospectBytecodeIsEOFBytecodeTest:testIsEOFBytecodeFuzz(bytes) (runs: 2048, μ: 3821, ~: 3803) | ||
| LibExtrospectBytecodeIsEOFBytecodeTest:testIsEOFBytecodeNonEOF() (gas: 432) | ||
| LibExtrospectBytecodeIsEOFBytecodeTest:testIsEOFBytecodeSingleByte() (gas: 397) | ||
| LibExtrospectBytecodeScanEVMOpcodesPresentInBytecodeTest:testScanEVMOpcodesPresentPush1() (gas: 855) | ||
| LibExtrospectBytecodeScanEVMOpcodesPresentInBytecodeTest:testScanEVMOpcodesPresentReference(bytes) (runs: 2048, μ: 41338, ~: 13073) | ||
| LibExtrospectBytecodeScanEVMOpcodesPresentInBytecodeTest:testScanEVMOpcodesPresentRevertsOnEOF() (gas: 4426) | ||
| LibExtrospectBytecodeScanEVMOpcodesPresentInBytecodeTest:testScanEVMOpcodesPresentSimple() (gas: 1019) | ||
| LibExtrospectBytecodeTryTrimSolidityCBORMetadataTest:testTryTrimSolidityCBORMetadataBytecodeContrived(bytes) (runs: 2048, μ: 7499, ~: 7303) | ||
| LibExtrospectBytecodeTryTrimSolidityCBORMetadataTest:testTryTrimSolidityCBORMetadataBytecodeShort(bytes) (runs: 2048, μ: 4183, ~: 4186) | ||
| LibExtrospectBytecodeTryTrimSolidityCBORMetadataTest:testTryTrimSolidityCBORMetadataRevertsOnEOF() (gas: 4482) | ||
| LibExtrospectBytecodeTryTrimSolidityCBORMetadataTest:testTryTrimSolidityCBORMetdataBytecodeReal() (gas: 4103) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxyGasFailLength() (gas: 365) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxyGasFailPrefix() (gas: 684) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxyGasFailSuffix() (gas: 663) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxyGasSuccess() (gas: 662) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxyLength(bytes) (runs: 2048, μ: 3920, ~: 3898) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxyPrefixFail(bytes,address) (runs: 2048, μ: 4381, ~: 4282) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxySlowFail(bytes) (runs: 2048, μ: 3952, ~: 3847) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxyLength(bytes) (runs: 2048, μ: 3924, ~: 3898) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxyPrefixFail(bytes,address) (runs: 2048, μ: 4380, ~: 4282) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxySlowFail(bytes) (runs: 2048, μ: 3941, ~: 3847) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxySlowSuccess(address) (runs: 2048, μ: 12367, ~: 12367) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxySuccess(address) (runs: 2048, μ: 1332, ~: 1332) | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxySuffixFail(bytes,address) (runs: 2048, μ: 4396, ~: 4299) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableInvalid() (gas: 1142) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableJumpdest() (gas: 1954) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachablePush1() (gas: 789) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachablePush4() (gas: 1024) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableReference(bytes) (runs: 2048, μ: 41242, ~: 12259) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableReportedFalsePositive() (gas: 1713108) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableReportedFalsePositiveBytecode() (gas: 1664573) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableReturn() (gas: 1122) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableRevert() (gas: 1120) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableSelfdestruct() (gas: 1099) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableSimple() (gas: 1144) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableStop() (gas: 962) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanMetamorphicMetadata() (gas: 36173) No newline at end of file | ||
| LibExtrospectERC1167ProxyTest:testIsERC1167ProxySuffixFail(bytes,address) (runs: 2048, μ: 4397, ~: 4299) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableInvalid() (gas: 1457) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableJumpdest() (gas: 2316) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachablePush1() (gas: 924) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachablePush4() (gas: 1114) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachablePushWhileHalted() (gas: 1129) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableReference(bytes) (runs: 2048, μ: 48922, ~: 14723) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableReportedFalsePositive() (gas: 1708863) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableReportedFalsePositiveBytecode() (gas: 1666694) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableReturn() (gas: 1414) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableRevert() (gas: 1435) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableRevertsOnEOF() (gas: 4384) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableSelfdestruct() (gas: 1436) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableSimple() (gas: 1252) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanEVMOpcodesReachableStop() (gas: 1264) | ||
| LibExtrospectScanEVMOpcodesReachableInBytecodeTest:testScanMetamorphicMetadata() (gas: 33268) No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for TLOAD references in the codebase
rg -i 'tload' --type solidity -A 3 -B 3Repository: rainlanguage/rain.extrospection
Length of output: 107
🏁 Script executed:
#!/bin/bash
# Search for TLOAD in sol files and test files
rg -i 'tload' -g '*.sol' -g '*.t.sol' -A 2 -B 2Repository: rainlanguage/rain.extrospection
Length of output: 1360
🏁 Script executed:
#!/bin/bash
# Search for testTLoad or test functions in ExtrospectionInterpreterTest
rg -i 'testtload|function test.*tload' -g '*.sol' -A 3 -B 1Repository: rainlanguage/rain.extrospection
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Find ExtrospectionInterpreterTest file
fd -t f ExtrospectionInterpreterTest -e sol
# Also check what test functions exist for opcodes like SLOAD, SSTORE
rg 'function test.*[Oo]or\(' -g '*.sol' | head -30Repository: rainlanguage/rain.extrospection
Length of output: 1320
🏁 Script executed:
#!/bin/bash
# Check the test file for TLOAD test functions and understand the pattern
cat test/src/concrete/Extrospection.interpreter.t.sol | grep -A 10 'testSLoadooor\|testTLoad\|TLOAD' | head -50Repository: rainlanguage/rain.extrospection
Length of output: 477
🏁 Script executed:
#!/bin/bash
# Search for TestTLoad contract or testTLoad function
rg -i 'testtload|TestTLoad' -g '*.sol' -A 5 -B 2Repository: rainlanguage/rain.extrospection
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Check all disallowed opcodes in IExtrospectInterpreterV1.sol and verify test coverage
rg '1 << uint256\(EVM_OP_' src/interface/IExtrospectInterpreterV1.sol -A 1 -B 1Repository: rainlanguage/rain.extrospection
Length of output: 1484
🏁 Script executed:
#!/bin/bash
# Get complete list of test functions in Extrospection.interpreter.t.sol
rg 'function test[A-Za-z]*ooor\(' test/src/concrete/Extrospection.interpreter.t.solRepository: rainlanguage/rain.extrospection
Length of output: 395
🏁 Script executed:
#!/bin/bash
# Check if there are Test contract classes for TLOAD
rg 'contract.*TestTLoad|contract.*Test.*[Tt]load' -g '*.sol'
# Also check for TLOAD references in the entire test suite
rg 'TLOAD|tload' test/src/concrete/Extrospection.interpreter.t.solRepository: rainlanguage/rain.extrospection
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Double check the complete list of disallowed opcodes in IExtrospectInterpreterV1.sol
rg 'EVM_OP_[A-Z]' src/interface/IExtrospectInterpreterV1.sol | grep -E '\(1 << uint256\(EVM_OP_' | sed 's/.*EVM_OP_//' | sed 's/).*//'Repository: rainlanguage/rain.extrospection
Length of output: 156
🏁 Script executed:
#!/bin/bash
# Check the full test file to see all test functions
wc -l test/src/concrete/Extrospection.interpreter.t.sol
# Get all test functions
rg 'function test' test/src/concrete/Extrospection.interpreter.t.sol | wc -lRepository: rainlanguage/rain.extrospection
Length of output: 131
🏁 Script executed:
#!/bin/bash
# Check the gas-snapshot for any mention of TLOAD or TSTORE tests
cat .gas-snapshot | grep -i 'tload\|tstore'Repository: rainlanguage/rain.extrospection
Length of output: 57
Add tests for EVM_OP_TLOAD and EVM_OP_TSTORE opcodes.
Both TLOAD and TSTORE are included in the disallowed opcodes in IExtrospectInterpreterV1.sol, but unlike SLOAD/SSTORE and other disallowed opcodes, there are no corresponding test functions (e.g., testTLoadooor or testTStoreooor) in the test suite. The gas snapshot confirms this absence—it contains entries for testSLoadooor and testSStoreooor but none for the transient storage opcodes. Add test coverage to verify the disallow logic for both TLOAD and TSTORE.
🤖 Prompt for AI Agents
In @.gas-snapshot around lines 1 - 63, Add missing tests for transient storage
opcodes by creating two new test functions (e.g., testTLoadooor and
testTStoreooor) in the ExtrospectionInterpreterTest suite mirroring the existing
testSLoadooor and testSStoreooor tests; these should execute bytecode that
performs EVM_OP_TLOAD and EVM_OP_TSTORE respectively and assert the interpreter
disallows them (revert/failure) per the disallowed list in
IExtrospectInterpreterV1.sol. Place the tests alongside other opcode tests in
ExtrospectionInterpreterTest, use the same structure/assertions as
testSLoadooor/testSStoreooor so they show up in the gas snapshot, and reference
the disallowed-opcode behavior defined in IExtrospectInterpreterV1.sol. Ensure
test names exactly match the pattern (testTLoadooor, testTStoreooor) so CI/gas
snapshot picks them up.
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
Bug Fixes
Chores