Skip to content

2025 07 16 721 balance#364

Merged
thedavidmeister merged 5 commits intomainfrom
2025-07-16-721-balance
Jul 16, 2025
Merged

2025 07 16 721 balance#364
thedavidmeister merged 5 commits intomainfrom
2025-07-16-721-balance

Conversation

@thedavidmeister
Copy link
Copy Markdown
Contributor

@thedavidmeister thedavidmeister commented Jul 16, 2025

Motivation

Solution

Checks

By submitting this for review, I'm confirming I've done the following:

  • made this PR as small as possible
  • unit-tested any new functionality
  • linked any relevant issues or PRs
  • included screenshots (if this involves a front-end change)

Summary by CodeRabbit

  • New Features

    • Added support for the "erc721-balance-of" opcode alongside the existing "uint256-erc721-balance-of," enabling enhanced ERC721 balance queries with improved metadata and execution.
    • Introduced a new opcode implementation that returns ERC721 balances as fixed decimal floats within the interpreter environment.
  • Bug Fixes

    • Updated related tests and function interfaces to use the latest stack item types and operand standards, improving consistency and correctness.
  • Tests

    • Reactivated and expanded the test suite for ERC721 balance operations, covering integrity checks, execution, and evaluation scenarios for both new and existing opcodes.
    • Added new tests verifying opcode integrity enforcement and runtime behavior, including failure cases.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 16, 2025

Walkthrough

The changes activate and update the "uint256-erc721-balance-of" opcode across the codebase. This includes enabling the opcode in the standard operations list, updating its handler and function pointers, refactoring its implementation and tests to use StackItem arrays, and modernizing the test suite to align with the latest interpreter interfaces. Additionally, a new "erc721-balance-of" opcode is introduced with corresponding implementation and tests. The lib/rain.interpreter.interface submodule reference is updated.

Changes

File(s) Change Summary
lib/rain.interpreter.interface Subproject commit reference updated to a new hash.
src/lib/op/LibAllStandardOps.sol Opcode count incremented from 40 to 42; reactivated "uint256-erc721-balance-of" opcode; added new "erc721-balance-of" opcode; updated handler, integrity, and run function pointers for both.
src/lib/op/erc721/uint256/LibOpUint256ERC721BalanceOf.sol Refactored referenceFn to use StackItem[] for input/output instead of uint256[].
src/lib/op/erc721/LibOpERC721BalanceOf.sol New library added implementing "erc721-balance-of" opcode with integrity, run, and reference functions.
test/src/lib/op/erc721/uint256/LibOpUint256ERC721BalanceOf.t.sol Uncommented and updated test contract to use StackItem[], OperandV2, and modern interfaces; re-enabled all tests.
test/src/lib/op/erc721/LibOpUint256ERC721BalanceOf.t.sol New test contract added for "erc721-balance-of" opcode covering integrity, runtime, evaluation, and failure cases.
.gas-snapshot Updated gas snapshot with minor numerical changes and added entries for new ERC721 balance opcodes and tests.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Interpreter
    participant LibOpERC721BalanceOf
    participant ERC721Token

    User->>Interpreter: Evaluate "erc721-balance-of" opcode with StackItem[] inputs
    Interpreter->>LibOpERC721BalanceOf: Call run(state, operand, stackTop)
    LibOpERC721BalanceOf->>ERC721Token: Call balanceOf(account)
    ERC721Token-->>LibOpERC721BalanceOf: Return balance (uint256)
    LibOpERC721BalanceOf->>LibDecimalFloat: Convert balance to decimal float
    LibOpERC721BalanceOf-->>Interpreter: Push result onto stack
    Interpreter-->>User: Return evaluation result
Loading
sequenceDiagram
    participant User
    participant Interpreter
    participant LibOpUint256ERC721BalanceOf
    participant ERC721Token

    User->>Interpreter: Evaluate "uint256-erc721-balance-of" opcode with StackItem[] inputs
    Interpreter->>LibOpUint256ERC721BalanceOf: Call referenceFn(state, operand, StackItem[] inputs)
    LibOpUint256ERC721BalanceOf->>ERC721Token: Call balanceOf(account)
    ERC721Token-->>LibOpUint256ERC721BalanceOf: Return balance (uint256)
    LibOpUint256ERC721BalanceOf-->>Interpreter: Return StackItem[] output (balance)
    Interpreter-->>User: Return evaluation result
Loading

Possibly related PRs

  • 2025 07 15 erc20 balance #361: Adds and updates ERC721 balance opcode and its test suite with StackItem-based interfaces, similar to this PR's changes.
  • 2025 07 15 allowance op #360: Activates and updates the "uint256-erc20-allowance" opcode and related tests, modifying analogous files and interfaces for a different token standard.
  • bump int #323: Updates the subproject commit reference for lib/rain.interpreter.interface in a similar manner, indicating related submodule update activity.

Suggested reviewers

  • findolor
  • hardyjosh

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between de6e23a and 24530a9.

⛔ Files ignored due to path filters (3)
  • src/generated/Rainterpreter.pointers.sol is excluded by !**/generated/**
  • src/generated/RainterpreterExpressionDeployer.pointers.sol is excluded by !**/generated/**
  • src/generated/RainterpreterParser.pointers.sol is excluded by !**/generated/**
📒 Files selected for processing (4)
  • .gas-snapshot (21 hunks)
  • src/lib/op/LibAllStandardOps.sol (6 hunks)
  • src/lib/op/erc721/LibOpERC721BalanceOf.sol (1 hunks)
  • test/src/lib/op/erc721/LibOpUint256ERC721BalanceOf.t.sol (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.interpreter#360
File: src/lib/op/erc20/LibOpERC20Allowance.sol:0-0
Timestamp: 2025-07-15T11:31:28.010Z
Learning: In the rainlanguage/rain.interpreter project, forge (Foundry's formatting tool) handles code formatting automatically, so formatting-related suggestions are not actionable.
Learnt from: thedavidmeister
PR: rainlanguage/rain.interpreter#360
File: src/lib/op/erc20/LibOpERC20Allowance.sol:35-36
Timestamp: 2025-07-15T11:31:10.098Z
Learning: In the rain.interpreter codebase, when working with ERC20 tokens that may not implement the optional decimals() function, the preference is to let the call fail explicitly rather than catching errors and defaulting to 18 decimals. This ensures correctness by avoiding potentially incorrect calculations with assumed decimal values.
src/lib/op/erc721/LibOpERC721BalanceOf.sol (1)
Learnt from: thedavidmeister
PR: rainlanguage/rain.interpreter#360
File: src/lib/op/erc20/LibOpERC20Allowance.sol:35-36
Timestamp: 2025-07-15T11:31:10.098Z
Learning: In the rain.interpreter codebase, when working with ERC20 tokens that may not implement the optional decimals() function, the preference is to let the call fail explicitly rather than catching errors and defaulting to 18 decimals. This ensures correctness by avoiding potentially incorrect calculations with assumed decimal values.
test/src/lib/op/erc721/LibOpUint256ERC721BalanceOf.t.sol (1)
Learnt from: thedavidmeister
PR: rainlanguage/rain.interpreter#360
File: src/lib/op/erc20/LibOpERC20Allowance.sol:35-36
Timestamp: 2025-07-15T11:31:10.098Z
Learning: In the rain.interpreter codebase, when working with ERC20 tokens that may not implement the optional decimals() function, the preference is to let the call fail explicitly rather than catching errors and defaulting to 18 decimals. This ensures correctness by avoiding potentially incorrect calculations with assumed decimal values.
🔇 Additional comments (16)
src/lib/op/LibAllStandardOps.sol (6)

40-40: LGTM: Import addition is correct

The import for LibOpERC721BalanceOf follows the established pattern and is correctly placed alphabetically within the ERC721 imports section.


109-109: LGTM: Length constant correctly updated

The increment from 40 to 42 correctly accounts for the two new ERC721 balance opcodes being added.


187-194: LGTM: Opcode metadata is well-defined

The metadata clearly distinguishes between the two ERC721 balance opcodes, with the uint256 variant emphasizing its return type difference from the float variant. The descriptions are informative and follow the established pattern.


405-408: LGTM: Operand handlers correctly configured

Both opcodes appropriately use handleOperandDisallowed since they don't accept operands, consistent with other similar ERC token opcodes in the codebase.


570-571: LGTM: Integrity function pointers are correctly referenced

The integrity function pointers correctly reference the appropriate library functions for both ERC721 balance opcodes.


683-684: LGTM: Runtime function pointers are correctly referenced

The runtime function pointers correctly reference the appropriate library functions for both ERC721 balance opcodes.

src/lib/op/erc721/LibOpERC721BalanceOf.sol (3)

15-19: LGTM: Integrity function correctly specified

The integrity function correctly specifies 2 inputs (token address and account address) and 1 output (balance), which matches the ERC721 balanceOf function signature.


21-37: LGTM: Run function is efficiently implemented

The assembly-optimized implementation correctly:

  • Reads inputs from the stack with proper memory safety
  • Converts uint256 values to addresses using the standard uint160 pattern
  • Uses lossless decimal conversion appropriate for ERC721 balances (which are always whole numbers)
  • Follows established stack manipulation patterns

39-53: LGTM: Reference function correctly implements StackItem interface

The reference function properly:

  • Uses the updated StackItem[] interface for inputs and outputs
  • Correctly unwraps and wraps stack items
  • Implements the same logic as the optimized run function
  • Follows established patterns for reference implementations
test/src/lib/op/erc721/LibOpUint256ERC721BalanceOf.t.sol (4)

26-39: LGTM: Integrity test comprehensively validates input/output requirements

The test correctly verifies that the opcode always requires exactly 2 inputs and produces 1 output, regardless of the provided parameters, which matches the ERC721 balanceOf function signature.


41-66: LGTM: Runtime test properly validates opcode execution

The test correctly:

  • Uses proper test setup with address etching
  • Validates lossless conversion before testing (important for ERC721 balances)
  • Uses appropriate mocking patterns with vm.mockCall and vm.expectCall
  • Leverages opReferenceCheck to ensure consistency between reference and optimized implementations

68-99: LGTM: Evaluation test provides comprehensive end-to-end coverage

The test effectively validates the complete opcode workflow:

  • Parses bytecode with proper address formatting
  • Validates lossless conversion assumption
  • Uses appropriate mocking for external calls
  • Verifies both the stack result and absence of key-value store changes

101-148: LGTM: Comprehensive negative test coverage

The negative tests thoroughly validate error conditions:

  • Tests all invalid input counts (0, 1, 3) with proper error expectations
  • Tests invalid output counts (0, 2)
  • Validates operand restrictions
  • Uses both direct testing and helper functions for comprehensive coverage
.gas-snapshot (3)

410-410: Performance improvement detected in ERC721 balance operation.

The gas consumption for testOpERC721BalanceOfEvalHappy shows a significant decrease from 610932 to 104853 gas units, which represents approximately an 83% improvement. This substantial optimization in the ERC721 balance operation is excellent.


168-421: Gas snapshot updates for ERC721 balance operations look good.

The gas snapshot correctly captures the performance metrics for the newly added ERC721 balance opcodes. The test entries are properly formatted and the gas consumption values appear reasonable for the operations being tested.


1-820: Gas snapshot updates are properly maintained.

The gas snapshot file has been appropriately updated to reflect the changes from the ERC721 balance opcode additions. The minor variations in gas consumption across existing tests are typical for gas snapshots and don't indicate any concerning performance regressions.


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@thedavidmeister thedavidmeister merged commit d2cc52b into main Jul 16, 2025
11 checks passed
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.

1 participant