Skip to content

disambiguator on log table #153

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 3 commits into from
Oct 5, 2025
Merged

disambiguator on log table #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1dbb693
disambiguator on log table
thedavidmeister Oct 5, 2025
254a4c6
update deployed address
thedavidmeister Oct 5, 2025
889badb
Merge branch 'main' into 2025-10-05-debug
thedavidmeister Oct 5, 2025
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/LibDecimalFloat.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type Float is bytes32;
library LibDecimalFloat {
using LibDecimalFloat for Float;

address constant LOG_TABLES_ADDRESS = 0xdc564999251875a2728Eec15962c5456C9259f54;
address constant LOG_TABLES_ADDRESS = 0x6421E8a23cdEe2E6E579b2cDebc8C2A514843593;
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 5, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the deployment at the new address and ensure table data integrity.

The LOG_TABLES_ADDRESS change is critical as this constant is used by pow10, log10, pow, and sqrt functions. Using an incorrect address or one containing invalid table data would break all logarithmic operations.

Please run the following verification steps:

  1. Verify the contract is deployed at the new address and check if it contains the expected disambiguator:

  2. Search for deployment scripts or tests that verify the table data:

Additionally, consider documenting the deployment process or adding a verification test that ensures the deployed contract at LOG_TABLES_ADDRESS contains the expected table data structure.

🏁 Script executed:

#!/bin/bash
# Verify deployment and check for LOG_TABLE_DISAMBIGUATOR presence

NEW_ADDR="0x6421E8a23cdEe2E6E579b2cDebc8C2A514843593"

# Check if contract is deployed
CODE=$(cast code "$NEW_ADDR" --rpc-url mainnet 2>/dev/null || echo "")
if [ -z "$CODE" ] || [ "$CODE" = "0x" ]; then
  echo "ERROR: No contract deployed at $NEW_ADDR"
  exit 1
else
  echo "✓ Contract deployed at $NEW_ADDR"
  echo "Code size: ${#CODE} bytes"
fi

# Try to read LOG_TABLE_DISAMBIGUATOR if it's exposed
# This assumes the contract has a public getter for the disambiguator
cast call "$NEW_ADDR" "LOG_TABLE_DISAMBIGUATOR()(bytes32)" --rpc-url mainnet 2>/dev/null || echo "Note: Could not read LOG_TABLE_DISAMBIGUATOR (may not be publicly exposed)"

Length of output: 73

🏁 Script executed:

#!/bin/bash
# Find deployment and verification scripts

echo "=== Deployment scripts ==="
fd -e sol -e js -e ts . | xargs rg -l "LOG_TABLES_ADDRESS|0x6421E8a23cdEe2E6E579b2cDebc8C2A514843593"

echo -e "\n=== Tests for table deployment ==="
fd -e sol -e js -e ts . | xargs rg -l "LibDecimalFloatDeploy|combinedTables"

echo -e "\n=== LOG_TABLE_DISAMBIGUATOR usage ==="
ast-grep --pattern 'LOG_TABLE_DISAMBIGUATOR'

Length of output: 633

LOG_TABLES_ADDRESS not deployed on mainnet

cast code shows no contract at 0x6421E8a23cdEe2E6E579b2cDebc8C2A514843593 on mainnet. This constant underpins pow10, log10, pow, and sqrt—it must point to a live log-table contract. Deploy the tables at this address, update the constant to the correct address, or clarify the target network. Add a test that fails if the address has no deployed code.


/// A zero valued float.
Float constant FLOAT_ZERO = Float.wrap(0);
Expand Down
11 changes: 9 additions & 2 deletions src/lib/deploy/LibDecimalFloatDeploy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,18 @@ import {LibDataContract, DataContractMemoryContainer} from "rain.datacontract/li
import {LibBytes} from "rain.solmem/lib/LibBytes.sol";
import {LibMemCpy, Pointer} from "rain.solmem/lib/LibMemCpy.sol";
import {DecimalFloat} from "../../concrete/DecimalFloat.sol";
import {LOG_TABLE_DISAMBIGUATOR} from "../table/LibLogTable.sol";

library LibDecimalFloatDeploy {
function combinedTables() internal pure returns (bytes memory) {
return
abi.encodePacked(LOG_TABLES, LOG_TABLES_SMALL, LOG_TABLES_SMALL_ALT, ANTI_LOG_TABLES, ANTI_LOG_TABLES_SMALL);
return abi.encodePacked(
LOG_TABLES,
LOG_TABLES_SMALL,
LOG_TABLES_SMALL_ALT,
ANTI_LOG_TABLES,
ANTI_LOG_TABLES_SMALL,
LOG_TABLE_DISAMBIGUATOR
);
Comment on lines +20 to +27
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 5, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

Clarify the collision scenario and consider alternative approaches.

The appended disambiguator successfully alters the deployment address, but the implementation raises some design questions:

  1. Versioning pattern: The constant suffix _1 suggests potential future values (_2, _3, etc.), which could become a maintenance burden. Each new value requires updating both the constant definition and deployment logic.

  2. Cost implications: The extra 32 bytes are stored in the data contract, increasing deployment gas costs. While minimal, this cost is paid on every deployment.

  3. Use case clarity: The comment states it avoids collisions "when we actually want distinct addresses," but it's unclear from the code what scenarios require multiple distinct deployments of the same table data.

Consider documenting the specific use case (e.g., "required when deploying to multiple chains with different table versions" or "enables parallel deployments for testing") to help future maintainers understand when and why this is needed.

Run the following script to check for any additional context about deterministic deployments and how the combined tables are consumed:

🏁 Script executed:

#!/bin/bash
# Description: Search for usages of combinedTables() and dataContract() to understand the deployment flow

# Search for calls to combinedTables() or dataContract()
rg -n --type=sol -A 5 -B 2 'combinedTables\(\)|dataContract\(\)' --glob '!src/lib/deploy/LibDecimalFloatDeploy.sol'

# Search for CREATE2 or deterministic deployment patterns
rg -n --type=sol -A 3 'CREATE2|create2|deterministic'

# Search for comments explaining the deployment strategy
rg -n --type=sol -B 2 -A 2 'collision|disambiguator|distinct address'

Length of output: 83

🏁 Script executed:

#!/bin/bash
# Search for calls to combinedTables() or dataContract() outside the deploy library
rg -n -g '*.sol' -C3 'combinedTables\(\)|dataContract\(\)' --glob '!src/lib/deploy/LibDecimalFloatDeploy.sol'
# Search for CREATE2 or create2 usage
rg -n -g '*.sol' -C3 'CREATE2|create2'
# Search for comments explaining collision or disambiguator usage
rg -n -g '*.sol' -C3 'collision|disambiguator|distinct address'

Length of output: 2725

Reconsider the disambiguator design in combinedTables

  • The _1 suffix in LOG_TABLE_DISAMBIGUATOR implies future versions, adding maintenance overhead.
  • Appending 32 bytes to the payload increases deployment gas; consider leveraging CREATE2’s salt instead of expanding the data.
  • Collision rationale is already documented in src/lib/table/LibLogTable.sol.
🤖 Prompt for AI Agents 
In src/lib/deploy/LibDecimalFloatDeploy.sol around lines 20 to 27, the
combinedTables payload currently appends LOG_TABLE_DISAMBIGUATOR (with a _1
suffix) which both bloats deployment calldata and embeds a version-like suffix;
remove the disambiguator from the abi.encodePacked return and stop using the _1
suffix constant here, then switch the uniqueness/collision mechanism to
derive/encode the distinguishing value into the CREATE2 salt at deployment time
instead of into the table payload; update any deployment call sites to pass the
same distinguishing bytes via the CREATE2 salt and remove or rename the
LOG_TABLE_DISAMBIGUATOR constant and its usages so documentation in
src/lib/table/LibLogTable.sol remains the canonical rationale for collision
handling.

}

function dataContract() internal pure returns (DataContractMemoryContainer) {
Expand Down
4 changes: 4 additions & 0 deletions src/lib/table/LibLogTable.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ int256 constant ANTILOG_IDX_LAST_INDEX = ANTILOG_IDX_CARDINALITY - 1;
uint256 constant LOG_TABLE_SIZE_BASE = LOG_MANTISSA_IDX_CARDINALITY / 10;
uint256 constant LOG_TABLE_SIZE_BYTES = LOG_TABLE_SIZE_BASE * 2;

/// @dev As we deterministically deploy the log tables, we can run into
/// collisions when we actually want distinct addresses.
bytes32 constant LOG_TABLE_DISAMBIGUATOR = keccak256("LOG_TABLE_DISAMBIGUATOR_1");
Comment on lines +22 to +24
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 5, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

LGTM with documentation suggestion.

The constant is correctly defined with a deterministic keccak256 hash. The use of _1 in the string suggests a versioning scheme, which aligns with the need to generate different addresses for different deployments.

Consider expanding the documentation to briefly mention the specific scenario(s) that require distinct addresses (e.g., "used when deploying different versions of log tables to the same chain" or "enables multiple test deployments without address conflicts").

🤖 Prompt for AI Agents 
In src/lib/table/LibLogTable.sol around lines 22 to 24, expand the comment for
LOG_TABLE_DISAMBIGUATOR to state the specific scenarios needing distinct
addresses (e.g., deploying different versions of log tables to the same chain or
running multiple test deployments) so future readers understand why the
disambiguator/version suffix exists; update the docstring to include one short
sentence listing those scenarios and keep the existing constant unchanged.


/// @dev https://icap.org.pk/files/per/students/exam/notices/log-table.pdf
library LibLogTable {
function toBytes(uint16[10][90] memory table) internal pure returns (bytes memory) {
Expand Down