Skip to content

log tables #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thedavidmeister merged 1 commit into from
Jul 29, 2025
Merged

log tables #84

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/lib/deploy/LibDecimalFloatDeploy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {LibBytes} from "rain.solmem/lib/LibBytes.sol";
import {LibMemCpy, Pointer} from "rain.solmem/lib/LibMemCpy.sol";
import {DecimalFloat} from "../../concrete/DecimalFloat.sol";

address constant LOG_TABLES_ADDRESS = 0x7A0D94F55792C434d74a40883C6ed8545E406D12;
address constant LOG_TABLES_ADDRESS = 0x295180b25A5059a2e7eC64272ba4F85047B4146A;
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jul 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

LOG_TABLES_ADDRESS is now orphaned

The constant was updated to 0x295180b25A5059a2e7eC64272ba4F85047B4146A but nothing in this file (or any imported symbol shown here) references it any longer. Retaining an unused, misleading constant makes the contract brittle and invites silent bugs when someone assumes it is the “single source of truth”.

Either:

  1. Delete the constant if it is genuinely unused, or
  2. Replace the hard-coded literal in the Yul call below with this constant so there is one canonical address.
🤖 Prompt for AI Agents 
In src/lib/deploy/LibDecimalFloatDeploy.sol at line 16, the constant
LOG_TABLES_ADDRESS is defined but not used anywhere in the file, making it
misleading and brittle. To fix this, either remove the LOG_TABLES_ADDRESS
constant entirely if it is not needed, or find the Yul call in the file that
uses the hard-coded address 0x295180b25A5059a2e7eC64272ba4F85047B4146A and
replace that literal with the LOG_TABLES_ADDRESS constant to maintain a single
source of truth.

Copy link
Copy Markdown
Contributor Author

@thedavidmeister thedavidmeister Jul 29, 2025

Choose a reason for hiding this comment

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

it is used in the concrete DecimalFloat contract

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jul 29, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

@thedavidmeister Thank you for the clarification! You're absolutely right. Looking at the search results, I can see that LOG_TABLES_ADDRESS is indeed used in the concrete DecimalFloat contract. I can see from the output that the DecimalFloat contract imports from LibDecimalFloatDeploy and would have access to this constant.

My initial assessment was incorrect - I was only looking at the immediate file context and missed that this constant serves as a shared reference point for the DecimalFloat contract. The constant is properly used across the codebase architecture, not orphaned as I initially suggested.

I apologize for the confusion in my earlier comment.

🏁 Script executed:

#!/bin/bash
# Search for usage of LOG_TABLES_ADDRESS in the codebase
echo "=== Searching for LOG_TABLES_ADDRESS usage ==="
rg "LOG_TABLES_ADDRESS" -A 2 -B 2

echo -e "\n=== Examining DecimalFloat contract ==="
fd "DecimalFloat.sol" --type f | head -5

echo -e "\n=== Content of DecimalFloat contract ==="
cat src/concrete/DecimalFloat.sol

Length of output: 12536

@thedavidmeister You're absolutely correct, and thank you for the correction!

The script output clearly shows that LOG_TABLES_ADDRESS is indeed actively used in the DecimalFloat contract:

  1. Imported: import {LOG_TABLES_ADDRESS} from "../lib/deploy/LibDecimalFloatDeploy.sol";
  2. Used in three functions:
    • pow10(): return a.pow10(LOG_TABLES_ADDRESS);
    • log10(): return a.log10(LOG_TABLES_ADDRESS);
    • pow(): return a.pow(b, LOG_TABLES_ADDRESS);

My initial assessment was incorrect - I was only looking at the immediate file context and missed the cross-file usage pattern. The constant serves as a proper shared reference point between the deployment library and the concrete contract implementation.

I apologize for the confusion in my earlier review comment. The code is correctly structured with the constant being used exactly as intended.

🧠 Learnings used 
Learnt from: rouzwelt
PR: rainlanguage/rain.math.float#83
File: src/concrete/DecimalFloat.sol:248-251
Timestamp: 2025-07-24T04:32:14.171Z
Learning: In the rainlanguage/rain.math.float project, functions in DecimalFloat.sol that return tuples from LibDecimalFloat calls must unpack the tuple into local variables before returning them (rather than returning directly) to maintain compatibility with Slither static analysis checks.

Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#58
File: src/concrete/DecimalFloat.sol:175-182
Timestamp: 2025-06-16T13:17:28.513Z
Learning: In the rainlanguage/rain.math.float codebase, there's an established naming convention where functions accepting a `Float` type parameter consistently use `float` as the parameter name, even though it shadows the type name. This pattern is used throughout `LibDecimalFloat.sol` and should be maintained for consistency in related contracts like `DecimalFloat.sol`.

Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#30
File: test/src/lib/LibDecimalFloat.gt.t.sol:33-36
Timestamp: 2025-04-25T03:58:01.307Z
Learning: In the rain.math.float library, all values of `Float` (which is a type alias for bytes32) are considered valid and can be safely used with methods like gt(), lt(), or eq() without causing reverts.

Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#64
File: src/concrete/DecimalFloat.sol:0-0
Timestamp: 2025-06-17T10:02:01.394Z
Learning: In the rain.math.float project, the Float type is designed with a specific bit layout: 224-bit signed coefficient and 32-bit signed exponent, represented as 32 bytes total. The LibDecimalFloat.unpack function returns values that are guaranteed to fit within int224 and int32 ranges due to this internal representation, making explicit range checks before casting unnecessary.

Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#64
File: src/concrete/DecimalFloat.sol:0-0
Timestamp: 2025-06-17T10:02:01.394Z
Learning: In the rain.math.float project, the Float type is designed with a specific bit layout: 224-bit signed coefficient and 32-bit signed exponent, represented as 32 bytes total. The LibDecimalFloat.unpack function returns values that are guaranteed to fit within int224 and int32 ranges due to this internal representation, making explicit range checks before casting unnecessary.

Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#57
File: crates/float/src/lib.rs:316-328
Timestamp: 2025-06-18T09:10:41.740Z
Learning: In the rain.math.float codebase, the user prefers implementing standard Rust traits (like Neg) rather than creating redundant public methods when the trait already provides the needed functionality. Float implements Copy, so reference usage with operators is not a concern.

Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#58
File: crates/float/src/lib.rs:201-232
Timestamp: 2025-06-16T13:14:38.431Z
Learning: In the rain.math.float Rust crate, the `execute_call` function already provides sufficient abstraction for EVM contract calls by handling execution boilerplate, error handling, and result processing. Individual methods like `lt`, `eq`, `gt` only need to handle their specific call encoding and result decoding, making further abstraction unnecessary.

Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#59
File: crates/float/src/lib.rs:233-242
Timestamp: 2025-06-17T10:17:56.205Z
Learning: In the rainlanguage/rain.math.float repository, the maintainer 0xgleb prefers to handle documentation additions and improvements in separate issues rather than inline with feature PRs.

Learnt from: rouzwelt
PR: rainlanguage/rain.math.float#76
File: test_js/float.test.ts:9-32
Timestamp: 2025-07-17T02:38:44.698Z
Learning: In the rainlanguage/rain.math.float repository, the user rouzwelt accepts non-null assertions in test files because tests should throw and fail immediately when something goes wrong, making it clear where the issue occurred.


library LibDecimalFloatDeploy {
function combinedTables() internal pure returns (bytes memory) {
Expand Down
Loading