Skip to content

fix: default to zero address for read operations when account not provided - #121

Merged
cristiam86 merged 1 commit into
genlayerlabs:mainfrom
albert-mr:fix/default-zero-address-for-reads
Nov 3, 2025
Merged

cristiam86 merged 1 commit into
genlayerlabs:mainfrom
albert-mr:fix/default-zero-address-for-reads

Conversation

@albert-mr

@albert-mr albert-mr commented Nov 3, 2025 •

Copy link
Copy Markdown
Contributor

Problem

readContract and simulateWriteContract throw "Details: 'from'" error when no account is provided, despite documentation stating account is optional.

Solution

Default to zero address (0x0000...0000) when account not provided, following Ethereum eth_call standards.

Changes

  • src/contracts/actions.ts: Added ?? zeroAddress fallback (lines 74, 123)
  • tests/client.test.ts: Added test coverage for zero address default

Testing

All tests pass (13/13). Verified with live contract on Studionet.

Fully backwards compatible - existing code with accounts continues to work.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced contract interaction stability by introducing a default fallback mechanism when account credentials are unavailable during read and write operations.

…vided

Previously, readContract and simulateWriteContract would pass undefined
as the 'from' parameter when no account was provided, causing RPC errors.

This fix defaults to the zero address (0x0000...0000) when no account is
specified, following Ethereum standards (eth_call behavior). This allows
read-only operations to work without requiring wallet connection.

Changes:
- src/contracts/actions.ts: Added ?? zeroAddress fallback for both
  readContract (L74) and simulateWriteContract (L123)
- tests/client.test.ts: Added test coverage for zero address default

Fixes the issue where users received "Details: 'from'" errors when
attempting to read contract state without providing an account.
@coderabbitai

coderabbitai Bot commented Nov 3, 2025 •

Copy link
Copy Markdown

Walkthrough

The pull request adds a fallback to zeroAddress in the senderAddress resolution within readContract and simulateWriteContract functions, ensuring a defined value is used when neither account nor client.account is provided. A corresponding test verifies this behavior.

Changes

Cohort / File(s) Summary
senderAddress fallback to zeroAddress
src/contracts/actions.ts
Modified readContract and simulateWriteContract to compute senderAddress as account?.address ?? client.account?.address ?? zeroAddress instead of account?.address ?? client.account?.address, adding a default zero address when no account is provided.
Test coverage for zeroAddress default
tests/client.test.ts
Added import for zeroAddress from viem and introduced test case validating that readContract uses zeroAddress as the from field when no account is provided on the client or overrides.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify that zeroAddress is the intended and correct default fallback for sender address in read and simulated write contexts
  • Confirm the test assertion properly validates the new behavior and doesn't mask any unintended side effects
  • Consider whether this fallback should be applied in other contract-related functions for consistency

Possibly related PRs

Suggested reviewers

  • cristiam86
  • epsjunior

Poem

🐰✨ A rabbit hops through zero lanes,
Where addresses need default chains,
No account? No fear! The address stands,
Zero and true, from safety's hands!
Contracts now complete! 🎯

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "fix: default to zero address for read operations when account not provided" accurately and concisely describes the main change in the changeset. The changes to both readContract and simulateWriteContract in src/contracts/actions.ts specifically add a fallback to zeroAddress when no account is provided, which is exactly what the title conveys. The title follows conventional commits format and is clear enough for a teammate scanning history to understand the primary change.
Description Check ✅ Passed The PR description covers all essential information required by the template: the problem being addressed (readContract throwing errors when account not provided), the solution implemented (defaulting to zero address), the specific changes made (with file paths and line numbers), and testing performed (all 13 tests passing). While the description uses a slightly different structure than the template (Problem/Solution/Changes/Testing format instead of the What/Why/Testing done format), the template itself indicates it can be modified to fit needs, and the core required information is present and complete.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
tests/client.test.ts (1)

173-196: Good test coverage for the zero address default behavior.

The test properly verifies that readContract defaults to zeroAddress when no account is provided on the client or as an override. The assertions are comprehensive and follow the existing test patterns.

Consider adding a similar test case for simulateWriteContract since it also received the zero address fallback change (line 123 in src/contracts/actions.ts).

it("should use zero address for simulateWriteContract when no account is provided", async () => {
  const client = createClient({
    chain: localnet,
    // No account provided on client
  });

  const contractAddress = "0x1234567890123456789012345678901234567890";
  await client.simulateWriteContract({
    // No account override either
    address: contractAddress as Address,
    functionName: "testFunction",
    args: ["arg1", "arg2"],
  });

  expect(lastGenCallParams).toEqual([
    {
      type: "write",
      to: contractAddress,
      from: zeroAddress, // Should default to zero address
      data: expect.any(String),
      transaction_hash_variant: TransactionHashVariant.LATEST_NONFINAL,
    },
  ]);
});
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8152c05 and ece9541.

📒 Files selected for processing (2)
  • src/contracts/actions.ts (2 hunks)
  • tests/client.test.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/client.test.ts (1)
src/client/client.ts (1)
  • createClient (86-134)
🔇 Additional comments (3)
src/contracts/actions.ts (2)

74-74: LGTM! Proper fallback for read operations.

The zero address fallback aligns with Ethereum eth_call standards and resolves the issue where read operations failed when no account was provided. The fallback chain is clear and handles all cases appropriately.


123-123: LGTM! Appropriate default for simulated writes.

The zero address fallback is appropriate for simulated write operations since they don't execute actual transactions. This mirrors the behavior of readContract and maintains consistency across call-based operations.

tests/client.test.ts (1)

8-8: LGTM! Necessary import for the new test.

The zeroAddress import from viem is correctly added to support the new test case.

@cristiam86
cristiam86 merged commit 6a1b47a into genlayerlabs:main Nov 3, 2025
2 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.

2 participants