fix: route DecimalFloat log table reads to the deployed Zoltu address (#189)

[thedavidmeister]

Summary

Fixes#189 (audit/H01 — silent numeric corruption in every transcendental call on production).

LibDecimalFloat.LOG_TABLES_ADDRESS was hardcoded to 0x6421E8a23cdEe2E6E579b2cDebc8C2A514843593, but the deployment script deterministically deploys log tables to 0xc51a14251b0dcF0ae24A96b7153991378938f5F5 (LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS). DecimalFloat's pow10/log10/pow/sqrtextcodecopy from the wrong address — extcodecopy on an empty address silently copies zeros, so the production contract returns garbage from every transcendental call.

What changed

• Removed the duplicate LOG_TABLES_ADDRESS constant from src/lib/LibDecimalFloat.sol.
• Updated all 8 callsites (4 in src/concrete/DecimalFloat.sol, 4 in test/src/concrete/*.t.sol) to use LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS directly.

Single source of truth — no future drift between the lib's table address and the deploy script's expected address.

Breaking change

External integrators that referenced LibDecimalFloat.LOG_TABLES_ADDRESS must switch to LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS. They were already broken by the bug — calls against the old constant returned zero-table garbage on every prod chain.

Deploy required

This change modifies the DecimalFloat concrete bytecode (the address loaded into pow10/log10/pow/sqrt changes), so the deterministic deploy address shifts. Trigger Manual sol artifacts with suite decimal-float after merging this PR (per CLAUDE.md deploy flow).

Test plan

• Compile clean.
• Non-fork test suites (419 tests across all Lib* and LibDecimalFloatImplementation*) all pass locally.
• CI rainix-sol-test (will fail on testDeployAddress / testExpectedCodeHashDecimalFloat until deploy constants are regenerated post-deploy).
• After deploy: update DECIMAL_FLOAT_CONTRACT_HASH and ZOLTU_DEPLOYED_DECIMAL_FLOAT_ADDRESS in LibDecimalFloatDeploy.sol.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Chores
  • Updated canonical deployment references and adjusted deployment workflow execution.
• Bug Fixes
  • Constructor now validates required runtime tables and reverts with a clear error if missing or mismatched.
• Tests
  • Added extensive constructor/deployment sanity tests and a dedicated check helper; standardized many tests to use a shared test harness.

[coderabbitai]

Warning

Rate limit exceeded

@thedavidmeister has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 35 minutes and 5 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2129e285-d367-4e1e-91d2-8265d887f4d7

📥 Commits

Reviewing files that changed from the base of the PR and between 8f062ff and baf33f2.

📒 Files selected for processing (5)

• .github/workflows/manual-sol-artifacts.yaml
• test/abstract/LogTest.sol
• test/src/concrete/DecimalFloat.constructor.t.sol
• test/src/lib/deploy/LibDecimalFloatDeploy.checkLogTablesDeployed.t.sol
• test/src/lib/deploy/LibDecimalFloatDeploy.t.sol

Walkthrough

This PR removes the legacy hardcoded log-tables address, adds a runtime codehash check and error for the deployed log-tables, updates deployment constants, wires DecimalFloat to validate and use the deployed tables address, and updates/adds tests and the test harness to etch and validate the expected runtime.

Changes

Address Source Migration

Layer / File(s)	Summary
Deployment Constants Update src/lib/deploy/LibDecimalFloatDeploy.sol	ZOLTU_DEPLOYED_DECIMAL_FLOAT_ADDRESS and DECIMAL_FLOAT_CONTRACT_HASH updated.
Error Type src/error/ErrDecimalFloat.sol	Adds LogTablesNotDeployed(address tablesAddress, bytes32 expectedCodehash, bytes32 actualCodehash) error.
Deployment Check src/lib/deploy/LibDecimalFloatDeploy.sol	Adds checkLogTablesDeployed() that reads codehash at ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS and reverts with LogTablesNotDeployed on mismatch.
Remove Legacy Constant src/lib/LibDecimalFloat.sol	Removes LOG_TABLES_ADDRESS constant.
Concrete Contract Wiring src/concrete/DecimalFloat.sol	Imports LibDecimalFloatDeploy, adds constructor calling checkLogTablesDeployed(), and updates pow10, log10, pow, sqrt to use ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS.
Test Harness Setup test/abstract/LogTest.sol	Adds setUp() that etches combined log-tables runtime into ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS and validates codehash; logTables() no longer deploys lazily.
Transcendental Tests test/src/concrete/DecimalFloat.log10.t.sol, ...pow.t.sol, ...pow10.t.sol, ...sqrt.t.sol	Updated to import LibDecimalFloatDeploy and use ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS for external helper calls.
Constructor & Deploy-check Tests test/src/concrete/DecimalFloat.constructor.t.sol, test/src/lib/deploy/LibDecimalFloatDeploy.checkLogTablesDeployed.t.sol	New tests added covering missing/wrong/correct runtime and mutation cases for the constructor and checkLogTablesDeployed().
Test Base Migration many test/src/concrete/*.t.sol → test/abstract/LogTest.sol	Migrate many tests from forge-std/Test to LogTest so they run with the deterministic etched log tables.
CI Workflow .github/workflows/manual-sol-artifacts.yaml	Adds --skip-simulation to the forge script deploy command.

Sequence Diagram(s)

sequenceDiagram
participant Deployer
participant DecimalFloat
participant LibDecimalFloatDeploy
participant DeployedLogTables
Deployer->>DecimalFloat: deploy (constructor)
DecimalFloat->>LibDecimalFloatDeploy: checkLogTablesDeployed()
LibDecimalFloatDeploy->>DeployedLogTables: extcodehash(ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS)
DeployedLogTables-->>LibDecimalFloatDeploy: codehash
alt codehash == expected
LibDecimalFloatDeploy-->>DecimalFloat: success
else mismatch
LibDecimalFloatDeploy-->>Deployer: revert LogTablesNotDeployed(address, expected, actual)
end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• audit/H02: missing extcodesize guard before extcodecopy on log tables #190: Implements a constructor-level runtime check and switches calls to use LibDecimalFloatDeploy’s deployed address, directly addressing the audit finding about hardcoded LOG_TABLES_ADDRESS.

Possibly related PRs

• rainlanguage/rain.math.float#43: Modifies DecimalFloat wrapper and log-tables address usage; strong code-level overlap with this PR.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main fix: routing DecimalFloat log table reads to the correct deployed Zoltu address, directly addressing the audit issue #189.
Linked Issues check	✅ Passed	The PR meets all primary coding objectives from audit issue #189: removes the hardcoded LOG_TABLES_ADDRESS constant, updates all 8 callsites to use LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS, adds constructor guard with checkLogTablesDeployed(), adds comprehensive tests for the guard, and updates deployment constants.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the log table address mismatch and preventing silent numeric corruption. Test infrastructure updates (LogTest base class, fork setup refactoring) are necessary to support the fix and verification; no unrelated changes present.
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.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 2026-05-09-189

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

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)

test/src/concrete/DecimalFloat.sqrt.t.sol (1)

17-27: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

This test is self-referential and won’t catch a bad table deployment.

At Line 20-23 both values are derived from paths that use the same table address, so a wrong/empty deployment can still produce matching (wrong) outputs and pass. Add an independent check (e.g., codehash assertion) before comparison.

Suggested guard in test

 function testSqrtDeployed(Float a) external {
DecimalFloat deployed = new DecimalFloat();
+ assertEq(+ LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS.codehash,+ LibDecimalFloatDeploy.LOG_TABLES_DATA_CONTRACT_HASH+ );
try this.sqrtExternal(a) returns (Float c) {
Float deployedC = deployed.sqrt(a);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/src/concrete/DecimalFloat.sqrt.t.sol` around lines 17 - 27, The test
testSqrtDeployed is comparing outputs from two callers that use the same
deployed table, so add an independent guard to ensure the deployed DecimalFloat
table is actually deployed before comparing results: after creating DecimalFloat
deployed = new DecimalFloat(); assert that address(deployed).codehash (or
extcodesize(address(deployed))) indicates non-empty code (e.g., codehash !=
bytes32(0)) to detect an empty/wrong deployment, and only then proceed to call
deployed.sqrt(a) and compare with this.sqrtExternal(a); keep the check before
calling deployed.sqrt and before the assertEq so a bad/empty deployment fails
the test early.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/concrete/DecimalFloat.sol`:
- Around line 233-255: Add a fail-fast check that the deployed log tables
contract exists and matches the expected codehash before any table reads in the
exposed functions pow10, log10, pow, and sqrt in DecimalFloat.sol: before
calling a.pow10(...)/a.log10(...)/a.pow(...)/a.sqrt(...), perform an extcodehash
(or similar) on LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS and
require it equals the known expected table codehash (revert with a clear error
like "InvalidLogTablesContract" if not), so the functions revert immediately
instead of silently proceeding with a missing/mismatched table.
---
Outside diff comments:
In `@test/src/concrete/DecimalFloat.sqrt.t.sol`:
- Around line 17-27: The test testSqrtDeployed is comparing outputs from two
callers that use the same deployed table, so add an independent guard to ensure
the deployed DecimalFloat table is actually deployed before comparing results:
after creating DecimalFloat deployed = new DecimalFloat(); assert that
address(deployed).codehash (or extcodesize(address(deployed))) indicates
non-empty code (e.g., codehash != bytes32(0)) to detect an empty/wrong
deployment, and only then proceed to call deployed.sqrt(a) and compare with
this.sqrtExternal(a); keep the check before calling deployed.sqrt and before the
assertEq so a bad/empty deployment fails the test early.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 37595689-9dac-452d-b514-7a56782c31de

📥 Commits

Reviewing files that changed from the base of the PR and between fac4efb and 0d9b846.

📒 Files selected for processing (7)

• src/concrete/DecimalFloat.sol
• src/lib/LibDecimalFloat.sol
• src/lib/deploy/LibDecimalFloatDeploy.sol
• test/src/concrete/DecimalFloat.log10.t.sol
• test/src/concrete/DecimalFloat.pow.t.sol
• test/src/concrete/DecimalFloat.pow10.t.sol
• test/src/concrete/DecimalFloat.sqrt.t.sol

💤 Files with no reviewable changes (1)

• src/lib/LibDecimalFloat.sol

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/src/concrete/DecimalFloat.constructor.t.sol`:
- Around line 74-77: The assembly create result `temp` is unchecked before
calling `vm.etch`, so if `create` failed the test setup is ambiguous; after the
assembly block that does `temp := create(0, add(creationCode, 0x20),
mload(creationCode))` add an explicit assertion that `temp` is non-zero (e.g.,
require/assert that `temp != address(0)` or `temp != 0`) before `vm.etch` using
the same `temp` and reference
`LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS` and `temp.code` to
ensure creation succeeded and fail fast with a clear message if it did not.
- Around line 84-85: The test is too broad using vm.expectRevert() before new
DecimalFloat(); — tighten it to assert the specific revert emitted by the
DecimalFloat constructor (the codehash guard) by replacing vm.expectRevert()
with vm.expectRevert(<expected>) where <expected> is the exact revert reason or
encoded selector used by the constructor (e.g., the revert string or
abi.encodeWithSelector/<encoded bytes> for the custom error). Use the
DecimalFloat constructor's actual error identifier (error name or message or
bytes4 selector) so the test fails only if that specific guard stops deployment.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a6144314-326a-41ba-bced-9d2b763d76b9

📥 Commits

Reviewing files that changed from the base of the PR and between 0d9b846 and e847d4a.

📒 Files selected for processing (33)

• src/concrete/DecimalFloat.sol
• src/error/ErrDecimalFloat.sol
• src/lib/deploy/LibDecimalFloatDeploy.sol
• test/abstract/LogTest.sol
• test/src/concrete/DecimalFloat.abs.t.sol
• test/src/concrete/DecimalFloat.add.t.sol
• test/src/concrete/DecimalFloat.ceil.t.sol
• test/src/concrete/DecimalFloat.constants.t.sol
• test/src/concrete/DecimalFloat.constructor.t.sol
• test/src/concrete/DecimalFloat.div.t.sol
• test/src/concrete/DecimalFloat.eq.t.sol
• test/src/concrete/DecimalFloat.floor.t.sol
• test/src/concrete/DecimalFloat.format.t.sol
• test/src/concrete/DecimalFloat.frac.t.sol
• test/src/concrete/DecimalFloat.fromFixedDecimalLossless.t.sol
• test/src/concrete/DecimalFloat.fromFixedDecimalLossy.t.sol
• test/src/concrete/DecimalFloat.gt.t.sol
• test/src/concrete/DecimalFloat.gte.t.sol
• test/src/concrete/DecimalFloat.integer.t.sol
• test/src/concrete/DecimalFloat.inv.t.sol
• test/src/concrete/DecimalFloat.isZero.t.sol
• test/src/concrete/DecimalFloat.lt.t.sol
• test/src/concrete/DecimalFloat.lte.t.sol
• test/src/concrete/DecimalFloat.max.t.sol
• test/src/concrete/DecimalFloat.min.t.sol
• test/src/concrete/DecimalFloat.minus.t.sol
• test/src/concrete/DecimalFloat.mul.t.sol
• test/src/concrete/DecimalFloat.parse.t.sol
• test/src/concrete/DecimalFloat.sub.t.sol
• test/src/concrete/DecimalFloat.toFixedDecimalLossless.t.sol
• test/src/concrete/DecimalFloat.toFixedDecimalLossy.t.sol
• test/src/lib/deploy/LibDecimalFloatDeploy.checkLogTablesDeployed.t.sol
• test/src/lib/deploy/LibDecimalFloatDeploy.t.sol

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/manual-sol-artifacts.yaml:
- Line 38: Split the single forge invocation so simulation remains enabled for
the "log-tables" suite but is skipped only for the "decimal-float" suite: call
the Deploy script twice (targeting script/Deploy.sol:Deploy) — first run for the
"log-tables" suite without the --skip-simulation flag, then run for the
"decimal-float" suite with --skip-simulation; update the workflow step in
manual-sol-artifacts.yaml to use two separate run commands (or otherwise pass a
suite selector to script/Deploy.sol to achieve the same) so only decimal-float
is run with --skip-simulation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1b5d01ae-c6fa-4b3e-aab4-ddc2b30e7dbd

📥 Commits

Reviewing files that changed from the base of the PR and between e847d4a and 8f062ff.

📒 Files selected for processing (3)

• .github/workflows/manual-sol-artifacts.yaml
• src/lib/deploy/LibDecimalFloatDeploy.sol
• test/src/lib/deploy/LibDecimalFloatDeploy.t.sol

[github-actions]

@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment:

S/M/L PR Classification Guidelines:

This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed.

Small (S)

Characteristics:

• Simple bug fixes, typos, or minor refactoring
• Single-purpose changes affecting 1-2 files
• Documentation updates
• Configuration tweaks
• Changes that require minimal context to review

Review Effort: Would have taken 5-10 minutes

Examples:

• Fix typo in variable name
• Update README with new instructions
• Adjust configuration values
• Simple one-line bug fixes
• Import statement cleanup

Medium (M)

Characteristics:

• Feature additions or enhancements
• Refactoring that touches multiple files but maintains existing behavior
• Breaking changes with backward compatibility
• Changes requiring some domain knowledge to review

Review Effort: Would have taken 15-30 minutes

Examples:

• Add new feature or component
• Refactor common utility functions
• Update dependencies with minor breaking changes
• Add new component with tests
• Performance optimizations
• More complex bug fixes

Large (L)

Characteristics:

• Major feature implementations
• Breaking changes or API redesigns
• Complex refactoring across multiple modules
• New architectural patterns or significant design changes
• Changes requiring deep context and multiple review rounds

Review Effort: Would have taken 45+ minutes

Examples:

• Complete new feature with frontend/backend changes
• Protocol upgrades or breaking changes
• Major architectural refactoring
• Framework or technology upgrades

Additional Factors to Consider

When deciding between sizes, also consider:

• Test coverage impact: More comprehensive test changes lean toward larger classification
• Risk level: Changes to critical systems bump up a size category
• Team familiarity: Novel patterns or technologies increase complexity

Notes:

• the assessment must be for the totality of the PR, that means comparing the base branch to the last commit of the PR
• the assessment output must be exactly one of: S, M or L (single-line comment) in format of: SIZE={S/M/L}
• do not include any additional text, only the size classification
• your assessment comment must not include tips or additional sections
• do NOT tag me or anyone else on your comment

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

SIZE=L