Skip to content

Conversation

@thedavidmeister
Copy link
Contributor

@thedavidmeister thedavidmeister commented Jan 27, 2026

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

  • Chores

    • Updated submodule pointers and repository metadata.
  • Refactor

    • Improved data storage/deployment logic, expanded inline documentation, added a configurable deployment proxy address, and tightened deployment failure handling.
  • Tests

    • Updated gas/performance snapshots and added tests covering proxy-related write failure and missing-proxy scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 27, 2026

Walkthrough

Updated submodule pointers for lib/forge-std and lib/rain.solmem; added ZOLTU_PROXY_ADDRESS and adjusted SSTORE2-related bytecode/prefix handling and deployment checks in src/lib/LibDataContract.sol; added tests testZoltuNoZoltu(bytes) and testZoltuBadZoltu(bytes) and small test helpers; updated .gas-snapshot metrics.

Changes

Cohort / File(s) Summary
Submodule updates
lib/forge-std, lib/rain.solmem
Updated submodule pointers: lib/forge-std1801b0541f4f..., lib/rain.solmem2e47e41af85a... (metadata-only).
Core library changes
src/lib/LibDataContract.sol
Added ZOLTU_PROXY_ADDRESS constant; expanded SSTORE2/bytecode documentation and BASE_PREFIX notes; adjusted prefix parsing and deployed code size calculations (subtract 1 to account for prepended 0x00); writeZoltu now uses a local zoltu variable (proxy) and treats zero deployed address or failed external call as an error.
Tests
test/lib/LibDataContract.t.sol
Imported WriteError and ZOLTU_PROXY_ADDRESS; added testZoltuNoZoltu(bytes) and testZoltuBadZoltu(bytes) and helper writeZoltuExternal(bytes); updated expected deterministic addresses; minor formatting tweaks.
Performance snapshot
.gas-snapshot
Updated μ/˜/gas values for existing tests and added new test entries (testZoltuNoZoltu(bytes), testZoltuBadZoltu(bytes)).

Sequence Diagram(s)

sequenceDiagram
    participant Test
    participant DataContract as LibDataContract
    participant ZoltuProxy as ZoltuProxy

    Test->>DataContract: call writeZoltuExternal(data)
    DataContract->>DataContract: build container, adjust prefix (-1)
    DataContract->>ZoltuProxy: external call to deploy (use ZOLTU_PROXY_ADDRESS)
    alt proxy returns deployedAddress != 0
        ZoltuProxy-->>DataContract: deployedAddress
        DataContract-->>Test: success (returns address)
    else proxy returns 0 or call fails
        ZoltuProxy-->>DataContract: 0 or revert
        DataContract--xTest: revert with WriteError
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • 2025 09 25 audit #11 — touches src/lib/LibDataContract.sol and relates to Zoltu deployment/address handling; likely code-level overlap.
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'update deps' is vague and does not accurately reflect the substantial changes made, which include updates to LibDataContract logic, new test functions, and gas snapshot changes beyond simple dependency updates. Consider using a more descriptive title that reflects the main changes, such as 'Add Zoltu proxy support and deployment error handling' or 'Implement Zoltu deployment proxy with enhanced error checking'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@lib/forge-std`:
- Line 1: Update the PR description for lib/forge-std to include motivation for
the dependency updates (e.g., bug fixes, features, security patches), a clear
list of the specific dependencies and versions changed along with the notable
changes in each, a summary of compatibility/testing performed
(unit/integration/testnet results), and any breaking changes or required
migration steps; place this content in the PR description so reviewers can
quickly locate it and reference the dependency list and testing notes when
reviewing.
- Line 1: Update the project's Solidity version constraints to satisfy forge-std
v1.14.0 by ensuring all contracts' pragma solidity lines (e.g., the top-level
pragma in any contracts referencing forge-std or test helpers) are at least
^0.8.13 or otherwise compatible, and run a quick compile/test pass to catch any
incompatibilities; additionally, expand the PR description to document why
forge-std and rain.solmem were bumped, list the breaking change (forge-std now
requires ^0.8.13), and summarize the rain.solmem audit/security fixes (Protofire
I01, L01, M01) and any required code adjustments or compatibility notes for
downstream consumers.

@@ -1 +1 @@
Subproject commit b8f065fda83b8cd94a6b2fec8fcd911dc3b444fd
Subproject commit 1801b0541f4fda118a10798fd3486bb7051c5dd6
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add context to the PR description.

The PR description is empty and provides no motivation or details about the dependency updates. Please add:

  • Why these dependencies are being updated (bug fixes, new features, security patches, etc.)
  • What specific changes or fixes are included
  • Whether any compatibility testing was performed
  • Any breaking changes or migration steps required

This context helps reviewers understand the changes and provides valuable documentation for future reference.

🤖 Prompt for AI Agents
In `@lib/forge-std` at line 1, Update the PR description for lib/forge-std to
include motivation for the dependency updates (e.g., bug fixes, features,
security patches), a clear list of the specific dependencies and versions
changed along with the notable changes in each, a summary of
compatibility/testing performed (unit/integration/testnet results), and any
breaking changes or required migration steps; place this content in the PR
description so reviewers can quickly locate it and reference the dependency list
and testing notes when reviewing.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Examine commits between old and new versions for both submodules

echo "=== forge-std changes ==="
echo "Old commit: b8f065fda83b8cd94a6b2fec8fcd911dc3b444fd"
echo "New commit: 1801b0541f4fda118a10798fd3486bb7051c5dd6"
echo ""

# Clone forge-std and check commit log
if git clone --quiet https://github.com/foundry-rs/forge-std.git /tmp/forge-std 2>&1; then
  cd /tmp/forge-std
  echo "Commits in forge-std between versions:"
  git log --oneline b8f065fda83b8cd94a6b2fec8fcd911dc3b444fd..1801b0541f4fda118a10798fd3486bb7051c5dd6 2>&1 || echo "Could not fetch commit range"
  echo ""
  echo "Commit subjects:"
  git log --pretty=format:"%h - %s" b8f065fda83b8cd94a6b2fec8fcd911dc3b444fd..1801b0541f4fda118a10798fd3486bb7051c5dd6 2>&1 | head -20
else
  echo "Failed to clone forge-std"
fi

echo ""
echo ""
echo "=== rain.solmem changes ==="
echo "Old commit: f28da2f09d42d154783cb53c138a7ef2f3a9eb4a"
echo "New commit: 2e47e41af85a711d837b2518656db0efdae814c2"
echo ""

# Try to find and clone rain.solmem
if git clone --quiet https://github.com/rainprotocol/rain.solmem.git /tmp/rain.solmem 2>&1 || git clone --quiet https://github.com/rainlanguage/rain.solmem.git /tmp/rain.solmem 2>&1; then
  if [ -d "/tmp/rain.solmem" ]; then
    cd /tmp/rain.solmem
    echo "Commits in rain.solmem between versions:"
    git log --oneline f28da2f09d42d154783cb53c138a7ef2f3a9eb4a..2e47e41af85a711d837b2518656db0efdae814c2 2>&1 || echo "Could not fetch commit range"
    echo ""
    echo "Commit subjects:"
    git log --pretty=format:"%h - %s" f28da2f09d42d154783cb53c138a7ef2f3a9eb4a..2e47e41af85a711d837b2518656db0efdae814c2 2>&1 | head -20
  fi
else
  echo "Failed to clone rain.solmem - may not be a public repository"
fi

Repository: rainlanguage/rain.datacontract

Length of output: 4901


🌐 Web query:

forge-std foundry breaking changes security advisory

💡 Result:

forge-std (Foundry) — breaking changes & security advisory status (as of Jan 27, 2026)

Security advisory / policy

  • No published security advisories are currently listed for foundry-rs/forge-std on GitHub. [1]
  • Vulnerability reports are directed to security@ithaca.xyz. [1]

Notable breaking changes in forge-std

  • v1.14.0: Solidity compatibility is now limited to ^0.8.13 (this is the first release with that requirement; “all future releases” also require ^0.8.13). For <0.8.13, use v1.13.0. [2]
  • v1.13.0: Marked as the final release compatible with Solidity versions <0.8.13. [3]
  • v1.9.6: Deprecated deployMockERC721 and deployMockERC20 from StdUtils. [4]

Sources: [1] [2] [3] [4]


🌐 Web query:

rain.solmem solidity breaking changes

💡 Result:

Key Solidity-version “breaking change” implications for rain.solmem are mostly about memory-safe inline assembly:

  • rain.solmem requires a Solidity version that supports the memory-safe assembly dialect: it uses assembly ("memory-safe") { ... } in its libs (e.g., LibBytes.sol), so you need Solidity ≥ 0.8.13 (when this syntax was introduced). The verified sources commonly pin around 0.8.18+ / 0.8.19. [1], [2]
  • Avoid Solidity 0.8.13 and 0.8.14 for production if you rely on inline assembly + optimizer: Solidity explicitly fixed an optimizer bug affecting inline assembly memory side-effects in 0.8.15 (bug present in 0.8.13–0.8.14). [3]
  • Upcoming breaking change (Solidity 0.9.0): the fallback annotation form /// @solidity memory-safe-assembly (the comment-based memory-safe marker) is planned to be removed, so any code still using that comment form must switch to assembly ("memory-safe") {}. rain.solmem already uses the dialect form, but downstream code might not. [2]

Sources:
[1] Solidity 0.8.13 introduced assembly ("memory-safe") { ... } (and the comment-based alternative). https://www.soliditylang.org/blog/2022/03/16/solidity-0.8.13-release-announcement/
[2] Solidity 0.8.31 announcement: comment-based memory-safe-assembly planned for removal in 0.9.0. https://www.soliditylang.org/blog/2025/12/03/solidity-0.8.31-release-announcement/
[3] Solidity 0.8.15 fixed an optimizer bug affecting inline assembly in 0.8.13–0.8.14. https://soliditylang.org/blog/2022/06/15/solidity-0.8.15-release-announcement/
[4] Example verified sources showing rain.solmem usage (LibBytes.sol uses assembly ("memory-safe") and pragma solidity ^0.8.18). https://etherscan.io/address/0xf1224a483ad7f1e9aa46a8ce41229f32d7549a74


Verify Solidity version compatibility with forge-std v1.14.0 requirement and document the breaking changes.

forge-std has been updated to v1.14.0, which introduces a breaking change: it now requires Solidity ^0.8.13 (previous versions supported earlier versions). Ensure the project's pragma solidity statement is compatible with this constraint.

The rain.solmem update includes important audit fixes and security findings resolutions (Protofire I01, L01, M01). However, the empty PR description provides no explanation for why these dependency updates are being made. Add a clear description detailing the motivation, the breaking changes introduced, and any compatibility requirements.

🤖 Prompt for AI Agents
In `@lib/forge-std` at line 1, Update the project's Solidity version constraints
to satisfy forge-std v1.14.0 by ensuring all contracts' pragma solidity lines
(e.g., the top-level pragma in any contracts referencing forge-std or test
helpers) are at least ^0.8.13 or otherwise compatible, and run a quick
compile/test pass to catch any incompatibilities; additionally, expand the PR
description to document why forge-std and rain.solmem were bumped, list the
breaking change (forge-std now requires ^0.8.13), and summarize the rain.solmem
audit/security fixes (Protofire I01, L01, M01) and any required code adjustments
or compatibility notes for downstream consumers.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/lib/LibDataContract.sol`:
- Around line 142-164: Remove the dead `success` variable in writeZoltu and the
unused assignment inside the assembly block: in function writeZoltu delete the
`bool success;` declaration and change the assembly call so it no longer assigns
to `success` (i.e., invoke call(...) without `success :=`). Keep the rest of the
assembly logic (mstore, reading length from mload(container), setting
deployedAddress via mload(0)) and the existing post-assembly check `if
(deployedAddress == address(0)) revert WriteError();`.

@thedavidmeister thedavidmeister merged commit 6217921 into main Jan 27, 2026
4 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