Skip to content

2025 07 15 erc20 balance#361

Merged
thedavidmeister merged 3 commits intomainfrom
2025-07-15-erc20-balance
Jul 15, 2025
Merged

2025 07 15 erc20 balance#361
thedavidmeister merged 3 commits intomainfrom
2025-07-15-erc20-balance

Conversation

@thedavidmeister
Copy link
Copy Markdown
Contributor

@thedavidmeister thedavidmeister commented Jul 15, 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

    • Reactivated and updated ERC20 balance query operations, supporting both floating-point and raw uint256 formats within the interpreter.
  • Refactor

    • Modernized internal logic to use floating-point decimal representation for ERC20 balances, improving accuracy and type safety.
  • Tests

    • Restored and enhanced test coverage for ERC20 balance operations, including improved overflow and error handling checks with updated types and error specificity.
  • Bug Fixes

    • Improved error reporting for lossy conversions and overflow scenarios during ERC20 balance evaluations.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 15, 2025

Walkthrough

This change re-enables and updates the ERC20 balanceOf opcode and its uint256 variant within the interpreter's standard opcode set. The opcodes are modernized to use new operand and stack item types, switch from fixed-point to floating-point decimal representations, and update all related tests and reference functions. Snapshot files and test helpers are also adjusted to reflect the changes.

Changes

File(s) Change Summary
.gas-snapshot Updated test run counts and gas/performance metrics for various tests; no code or logic changes.
src/lib/op/LibAllStandardOps.sol Re-enabled and integrated LibOpERC20BalanceOf and LibOpUint256ERC20BalanceOf into standard ops; updated function pointers/types.
src/lib/op/erc20/LibOpERC20BalanceOf.sol Refactored to use float-based decimal conversion, new operand/stack types, and updated reference function interface.
src/lib/op/erc20/uint256/LibOpUint256ERC20BalanceOf.sol Updated reference function to use StackItem arrays instead of uint256 arrays.
test/abstract/OpTest.sol Added helper for checking CoefficientOverflow errors in tests.
test/src/lib/op/erc20/LibOpERC20BalanceOf.t.sol Uncommented and modernized tests for ERC20 balanceOf opcode; updated to new types and error checks.
test/src/lib/op/erc20/uint256/LibOpUint256ERC20BalanceOf.t.sol Uncommented and updated tests for uint256 ERC20 balanceOf opcode; aligned with new interfaces and types.

Sequence Diagram(s)

sequenceDiagram
    participant Interpreter
    participant LibAllStandardOps
    participant LibOpERC20BalanceOf
    participant ERC20Token

    Interpreter->>LibAllStandardOps: Execute opcode (erc20-balance-of)
    LibAllStandardOps->>LibOpERC20BalanceOf: Call run(state, operand, stackTop)
    LibOpERC20BalanceOf->>ERC20Token: Call balanceOf(account)
    ERC20Token-->>LibOpERC20BalanceOf: Return balance (uint256)
    LibOpERC20BalanceOf->>LibOpERC20BalanceOf: Convert balance to Float (decimal float)
    LibOpERC20BalanceOf-->>LibAllStandardOps: Return Float on stack
    LibAllStandardOps-->>Interpreter: Continue execution
Loading

Possibly related PRs

  • 2025 07 15 allowance op #360: Re-enables and updates the ERC20 allowance opcodes and their tests, following similar patterns for opcode integration and modernization as this PR but for a different ERC20 operation.

📜 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 94c46fb and 393037f.

⛔ 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 (7)
  • .gas-snapshot (21 hunks)
  • src/lib/op/LibAllStandardOps.sol (7 hunks)
  • src/lib/op/erc20/LibOpERC20BalanceOf.sol (1 hunks)
  • src/lib/op/erc20/uint256/LibOpUint256ERC20BalanceOf.sol (2 hunks)
  • test/abstract/OpTest.sol (2 hunks)
  • test/src/lib/op/erc20/LibOpERC20BalanceOf.t.sol (1 hunks)
  • test/src/lib/op/erc20/uint256/LibOpUint256ERC20BalanceOf.t.sol (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.interpreter#360
File: src/lib/op/erc20/LibOpERC20Allowance.sol:35-36
Timestamp: 2025-07-15T11:31:10.064Z
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/erc20/LibOpERC20BalanceOf.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.064Z
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/abstract/OpTest.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.064Z
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/erc20/LibOpERC20BalanceOf.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.064Z
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.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: git-clean
  • GitHub Check: rainix (macos-latest, rainix-rs-artifacts)
🔇 Additional comments (23)
test/abstract/OpTest.sol (2)

28-28: Good addition for enhanced error checking.

The import of CoefficientOverflow alongside ExponentOverflow supports more granular error handling in the updated decimal float system.


268-272: Well-structured helper function.

The new checkUnhappyCoefficientOverflow function follows the established pattern of the existing checkUnhappyOverflow helper and provides consistent error checking for coefficient overflow conditions.

src/lib/op/erc20/uint256/LibOpUint256ERC20BalanceOf.sol (1)

35-46: Proper modernization to StackItem types.

The function signature update and implementation correctly handle the transition from raw uint256[] to StackItem[] types. The unwrapping of inputs and wrapping of outputs is implemented correctly.

.gas-snapshot (1)

1-779: Normal gas snapshot update.

The gas snapshot changes reflect the performance impact of the opcode modernization and test suite updates. The variations in run counts and gas costs are expected when opcodes are re-enabled and tests are updated.

src/lib/op/erc20/LibOpERC20BalanceOf.sol (3)

16-20: Correct function signature update.

The integrity function properly uses OperandV2 instead of the older Operand type, maintaining the same input/output counts for the opcode.


30-41: Excellent modernization to floating-point decimals.

The implementation correctly:

  • Queries the ERC20 balance as before
  • Retrieves token decimals using IERC20Metadata.decimals()
  • Converts to Float using LibDecimalFloat.fromFixedDecimalLosslessPacked
  • Stores the Float result on the stack

The approach of letting the decimals() call fail explicitly (rather than defaulting to 18) aligns with the codebase preference for correctness over convenience.


43-59: Proper StackItem integration in reference function.

The referenceFn correctly:

  • Accepts and returns StackItem[] instead of uint256[]
  • Unwraps inputs to extract token and account addresses
  • Performs the same balance and decimals conversion as the run function
  • Wraps the Float result back into a StackItem

The implementation is consistent with the modernization effort and maintains correctness.

src/lib/op/LibAllStandardOps.sol (5)

36-36: Re-enabling ERC20 balanceOf opcode import

Good to see the systematic re-enablement of the ERC20 balanceOf opcode. The import is properly uncommented and aligns with the other active opcode imports.


108-108: Verify the opcode count increase

The count increase from 36 to 38 correctly reflects the addition of two opcodes (uint256-erc20-balance-of and erc20-balance-of).


167-170: AuthoringMetaV2 entries properly uncommented

The documentation entries for both ERC20 balanceOf opcodes are clear and provide appropriate descriptions for users. The uint256 variant correctly notes it returns a uint256 value, while the float variant doesn't specify this detail.

Also applies to: 179-182


390-391: Operand handlers correctly set to disallowed

Both opcodes properly use handleOperandDisallowed since they don't accept operands, which is consistent with their expected usage pattern.

Also applies to: 396-397


558-561: Function pointers correctly uncommented

The integrity and run function pointers are properly added back to their respective arrays, maintaining the correct ordering with other opcodes.

Also applies to: 670-673

test/src/lib/op/erc20/uint256/LibOpUint256ERC20BalanceOf.t.sol (5)

4-11: Imports properly modernized

The imports have been correctly updated to use the new types (StackItem, OperandV2) and remove unused legacy imports. This aligns with the modernization of the opcode implementation.


16-21: Integrity test properly updated

The integrity test correctly uses OperandV2 and validates the expected input/output counts (2 inputs, 1 output) for the ERC20 balanceOf operation.


23-44: Run test modernized with proper type usage

The test correctly uses StackItem[] for inputs and wraps addresses appropriately. The mock setup and reference check are comprehensive.


47-54: Happy path evaluation test is comprehensive

The test properly validates the opcode parsing and execution with mocked ERC20 calls.


57-82: Error condition tests provide good coverage

The tests cover all critical error scenarios including wrong input counts, wrong output counts, and disallowed operands. This ensures robust error handling.

test/src/lib/op/erc20/LibOpERC20BalanceOf.t.sol (6)

4-14: Imports correctly updated for float decimal handling

The imports properly include the new decimal float libraries (LibDecimalFloat, Float, LossyConversionToFloat) needed for the modernized float-based decimal representation.


19-24: Integrity test properly modernized

The integrity test correctly uses OperandV2 and validates the expected input/output counts for the float variant of the ERC20 balanceOf operation.


26-58: Run test correctly handles decimal precision

The test properly assumes lossless conversion and mocks both balanceOf and decimals calls. This approach aligns with the retrieved learning about explicit failure when decimals() is not available, rather than defaulting to 18 decimals.


61-75: Happy path test properly handles decimal float conversion

The test correctly uses LibDecimalFloat.fromFixedDecimalLosslessPacked and unwraps the Float type for comparison. The assumption of lossless conversion ensures test reliability.


78-94: Overflow test comprehensively handles lossy conversion scenarios

The test properly validates the LossyConversionToFloat error case with specific coefficient and exponent parameters. The assumptions ensure the test covers the intended overflow scenario.


97-122: Error condition tests maintain comprehensive coverage

The tests cover all critical error scenarios including wrong input/output counts and disallowed operands, ensuring robust error handling for the float variant.


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 c7c4e47 into main Jul 15, 2025
11 checks passed
This was referenced Jul 15, 2025
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