Skip to content

add docs and refactor #70

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
0xgleb merged 2 commits into from
Jul 3, 2025
Merged

add docs and refactor #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fac756c
add docs
0xgleb Jun 30, 2025
74706bf
split up into modules and add docs
0xgleb Jul 3, 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
7 changes: 7 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/float/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,6 @@ getrandom = { version = "0.2.11", features = ["js", "js-sys"] }

[dev-dependencies]
alloy = { workspace = true, features = ["sol-types", "json-rpc", "arbitrary"] }
anyhow = "1.0.98"
proptest.workspace = true
serde_json = "1.0.140"
64 changes: 64 additions & 0 deletions crates/float/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
use alloy::primitives::{Bytes, FixedBytes};
use alloy::sol_types::SolError;
use revm::context::result::{EVMError, HaltReason, Output, SuccessReason};
use std::thread::AccessError;
use thiserror::Error;

use crate::DecimalFloat;

#[derive(Debug, Error)]
pub enum FloatError {
#[error("EVM error: {0}")]
Evm(#[from] EVMError<std::convert::Infallible>),
#[error("Float execution reverted with output: {0}")]
Revert(Bytes),
#[error("Float execution halted with reason: {0:?}")]
Halt(HaltReason),
#[error("Execution ended for non-return reason. Reason: {0:?}. Output: {1:?}")]
UnexpectedSuccess(SuccessReason, Output),
#[error(transparent)]
AlloySolTypes(#[from] alloy::sol_types::Error),
#[error("Decimal Float error: {0:?}")]
DecimalFloat(DecimalFloat::DecimalFloatErrors),
#[error("Decimal Float error selector: {0:?}")]
DecimalFloatSelector(Result<DecimalFloatErrorSelector, FixedBytes<4>>),
#[error(transparent)]
Access(#[from] AccessError),
}

#[derive(Debug)]
pub enum DecimalFloatErrorSelector {
CoefficientOverflow,
ExponentOverflow,
Log10Negative,
Log10Zero,
LossyConversionFromFloat,
NegativeFixedDecimalConversion,
WithTargetExponentOverflow,
}

impl TryFrom<FixedBytes<4>> for DecimalFloatErrorSelector {
type Error = FixedBytes<4>;

fn try_from(error_selector: FixedBytes<4>) -> Result<Self, Self::Error> {
let FixedBytes(bytes) = error_selector;
match bytes {
<DecimalFloat::CoefficientOverflow as SolError>::SELECTOR => {
Ok(Self::CoefficientOverflow)
}
<DecimalFloat::ExponentOverflow as SolError>::SELECTOR => Ok(Self::ExponentOverflow),
<DecimalFloat::Log10Negative as SolError>::SELECTOR => Ok(Self::Log10Negative),
<DecimalFloat::Log10Zero as SolError>::SELECTOR => Ok(Self::Log10Zero),
<DecimalFloat::LossyConversionFromFloat as SolError>::SELECTOR => {
Ok(Self::LossyConversionFromFloat)
}
<DecimalFloat::NegativeFixedDecimalConversion as SolError>::SELECTOR => {
Ok(Self::NegativeFixedDecimalConversion)
}
<DecimalFloat::WithTargetExponentOverflow as SolError>::SELECTOR => {
Ok(Self::WithTargetExponentOverflow)
}
_ => Err(error_selector),
}
}
}
63 changes: 63 additions & 0 deletions crates/float/src/evm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
use alloy::primitives::{Address, Bytes};
use alloy::sol_types::SolInterface;
use revm::context::result::{ExecutionResult, Output, SuccessReason};
use revm::context::{BlockEnv, CfgEnv, Evm, TxEnv};
use revm::database::InMemoryDB;
use revm::handler::EthPrecompiles;
use revm::handler::instructions::EthInstructions;
use revm::interpreter::interpreter::EthInterpreter;
use revm::primitives::address;
use revm::{Context, MainBuilder, MainContext, SystemCallEvm};
use std::cell::RefCell;

use crate::{DecimalFloat, FloatError};

/// Fixed address where the DecimalFloat contract is deployed in the in-memory EVM.
/// This arbitrary address is used consistently across all Calculator instances.
pub(crate) const FLOAT_ADDRESS: Address = address!("00000000000000000000000000000000000f10a2");

type EvmContext = Context<BlockEnv, TxEnv, CfgEnv, InMemoryDB>;
type LocalEvm = Evm<EvmContext, (), EthInstructions<EthInterpreter, EvmContext>, EthPrecompiles>;

thread_local! {
pub(crate) static LOCAL_EVM: RefCell<LocalEvm> = {
let mut db = InMemoryDB::default();
let bytecode = revm::state::Bytecode::new_legacy(DecimalFloat::DEPLOYED_BYTECODE.clone());
let account_info = revm::state::AccountInfo::default().with_code(bytecode);
db.insert_account_info(FLOAT_ADDRESS, account_info);

let evm = Context::mainnet().with_db(db).build_mainnet();
RefCell::new(evm)
};
}

pub(crate) fn execute_call<F, T>(calldata: Bytes, process_output: F) -> Result<T, FloatError>
where
F: FnOnce(Bytes) -> Result<T, FloatError>,
{
let result = LOCAL_EVM.try_with(|evm| {
let evm = &mut *evm.borrow_mut();
let result_and_state = evm.transact_system_call_finalize(FLOAT_ADDRESS, calldata)?;

Ok::<_, FloatError>(result_and_state.result)
})??;
Comment on lines +38 to +43
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jul 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Consider adding a comment to clarify the error propagation pattern.

The double ? operator pattern on line 43 might be confusing to readers. The first ? propagates any AccessError from try_with, and the second propagates the FloatError from the closure.

 pub(crate) fn execute_call<F, T>(calldata: Bytes, process_output: F) -> Result<T, FloatError>
 where
     F: FnOnce(Bytes) -> Result<T, FloatError>,
 {
+    // try_with can fail with AccessError if the thread local is already borrowed
+    // The inner closure can fail with FloatError from EVM execution
     let result = LOCAL_EVM.try_with(|evm| {
         let evm = &mut *evm.borrow_mut();
         let result_and_state = evm.transact_system_call_finalize(FLOAT_ADDRESS, calldata)?;
 
         Ok::<_, FloatError>(result_and_state.result)
     })??;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let result = LOCAL_EVM.try_with(|evm| {
let evm = &mut *evm.borrow_mut();
let result_and_state = evm.transact_system_call_finalize(FLOAT_ADDRESS, calldata)?;
Ok::<_, FloatError>(result_and_state.result)
})??;
pub(crate) fn execute_call<F, T>(calldata: Bytes, process_output: F) -> Result<T, FloatError>
where
F: FnOnce(Bytes) -> Result<T, FloatError>,
{
// try_with can fail with AccessError if the thread local is already borrowed
// The inner closure can fail with FloatError from EVM execution
let result = LOCAL_EVM.try_with(|evm| {
let evm = &mut *evm.borrow_mut();
let result_and_state = evm.transact_system_call_finalize(FLOAT_ADDRESS, calldata)?;
Ok::<_, FloatError>(result_and_state.result)
})??;
// ...
}
🤖 Prompt for AI Agents 
In crates/float/src/evm.rs around lines 38 to 43, add a comment above the line
with the double `?` operators explaining that the first `?` handles propagation
of `AccessError` from `try_with`, and the second `?` propagates the `FloatError`
from the closure. This will clarify the error handling flow for future readers.

Copy link
Copy Markdown
Contributor Author

@0xgleb 0xgleb Jul 3, 2025

Choose a reason for hiding this comment

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

this doesn't seem to match the description of try_with

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jul 3, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

@0xgleb, you're absolutely right to question my description. Let me check the actual behavior of try_with to provide an accurate explanation.

🏁 Script executed:

#!/bin/bash
# Search for the try_with method usage and documentation
rg "try_with" -A 5 -B 5

Length of output: 679

🏁 Script executed:

#!/bin/bash
# Look at the error types and From implementations to understand the conversion
rg "AccessError" -A 3 -B 3
# Check the FloatError enum to see the Access variant
rg "enum FloatError" -A 10

Length of output: 1614

🌐 Web query:

Rust std::thread::LocalKey try_with method documentation behavior AccessError

💡 Result:

The std::thread::LocalKey::try_with method in Rust provides a way to safely access the value associated with a thread-local storage (TLS) key and gracefully handle cases where the value is not available. Here is a detailed description of its behavior, with particular focus on the AccessError type:

Behavior of try_with:

  • try_with accepts a closure and attempts to execute it with a reference to the thread-local value, if that value is accessible.
  • It returns a Result<R, AccessError>, where R is the return type of the closure.

When does try_with return an AccessError?

  • AccessError is returned if the thread-local value is not accessible, specifically if the thread-local key has already been destroyed (for example, during or after the thread's destruction phase, such as in a destructor)[10][4][1].
  • This is different from a typical panic: it allows you to handle the error without unwinding the stack.
  • Note: If the key’s initializer itself panics while being called, try_with will panic rather than return AccessError; AccessError is only used for the destroyed/unavailable state[10].

AccessError details:

  • AccessError is a non-exhaustive struct, meaning in future versions it could have additional variants or fields[10][4].
  • Its debug formatter displays as AccessError, and its display formatter outputs "already destroyed"[10][4][1].
  • It implements Debug, Display, Clone, Copy, Eq, PartialEq, and Error[10][4][1].

Example Usage:

thread_local! {
    static FOO: String = String::from("hello");
}

let result = FOO.try_with(|val| val.len());
// result: Result<usize, std::thread::AccessError>

If FOO is still valid, this will return Ok(length). If it is already destroyed (e.g., during TLS destructor), it returns Err(AccessError)[10].

Summary Table:

Condition try_with Result
Key accessible and available Ok(value)
Key has been destroyed / already inaccessible Err(AccessError)
Key initializer panics during initialization Panic (does not return Err)

Relevant documentation snippets:

This function will return an [AccessError] if the key has been destroyed (typically, this only happens if you try to access a TLS value in a destructor)[10].

AccessError display: "already destroyed"[10][4][1].

In summary, LocalKey::try_with is the correct way to safely query a TLS value and can be used to avoid panics in drop-time code where the key might have been destroyed. If you need access to the value and can’t tolerate failure, use the with method, which panics on error instead[10][4].

Citations:

@0xgleb, you're absolutely correct! My description was inaccurate.

Looking at the documentation, try_with returns an AccessError when the thread-local key has been destroyed (typically during thread destruction/cleanup), not when it's already borrowed. The borrowing conflicts would be handled by the borrow_mut() call inside the closure, which would panic rather than return an error.

The correct explanation for the double ? pattern is:

  • First ?: Converts AccessError from try_with (when the thread-local key is destroyed) to FloatError::Access via the From implementation
  • Second ?: Propagates the FloatError from the inner closure

So a more accurate comment would be:

// try_with can fail with AccessError if the thread local key has been destroyed
// The inner closure can fail with FloatError from EVM execution
let result = LOCAL_EVM.try_with(|evm| {
    // ... rest of the code
})??;

Thank you for catching that mistake! The original suggestion about borrowing was incorrect.

🧠 Learnings used 
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#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#59
File: crates/float/src/lib.rs:447-461
Timestamp: 2025-06-17T10:11:32.740Z
Learning: In the rainlanguage/rain.math.float repository, the user 0xgleb prefers using .unwrap() over ? in tests because it provides clearer stack traces showing the exact line where panics occurred, making debugging easier.

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: 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.

Copy link
Copy Markdown
Contributor Author

@0xgleb 0xgleb Jul 3, 2025

Choose a reason for hiding this comment

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

I think it's clear enough that when two question marks are used that that's due to nested Result types

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jul 3, 2025

Choose a reason for hiding this comment

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

@0xgleb, you're absolutely right! The double ? pattern is indeed a well-established Rust idiom that experienced developers will immediately recognize as handling nested Result types. Adding a comment would be redundant for such a common pattern.

Thank you for the correction on the try_with behavior earlier as well - it's a good reminder to verify the specifics before making assumptions.

🐰✨

✏️ Learnings added 
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#70
File: crates/float/src/evm.rs:38-43
Timestamp: 2025-07-03T11:20:50.441Z
Learning: In the rainlanguage/rain.math.float codebase, the user 0xgleb prefers not to add explanatory comments for well-established Rust idioms like the double `?` pattern, as these are self-explanatory to experienced Rust developers and don't need over-commenting.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
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#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#59
File: crates/float/src/lib.rs:447-461
Timestamp: 2025-06-17T10:11:32.740Z
Learning: In the rainlanguage/rain.math.float repository, the user 0xgleb prefers using .unwrap() over ? in tests because it provides clearer stack traces showing the exact line where panics occurred, making debugging easier.

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: 0xgleb
PR: rainlanguage/rain.math.float#58
File: crates/float/src/lib.rs:382-401
Timestamp: 2025-06-16T13:19:32.009Z
Learning: In the rainlanguage/rain.math.float codebase, variable shadowing in property tests like test_lt_eq_gt_with_add is acceptable when it represents a logical progression of the same conceptual value being tested through different states.

Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#57
File: crates/float/src/lib.rs:558-574
Timestamp: 2025-06-18T09:20:39.225Z
Learning: In Rust, when the `Neg` trait is implemented for a custom type, the unary `-` operator simply delegates to the `neg()` method. Testing the `neg()` method directly is sufficient since there's no separate code path for the operator.

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.


match result {
ExecutionResult::Success {
reason: SuccessReason::Return,
output: Output::Call(output),
..
} => process_output(output),
ExecutionResult::Success { reason, output, .. } => {
Err(FloatError::UnexpectedSuccess(reason, output))
}
ExecutionResult::Revert { output, .. } => {
if let Ok(error) = DecimalFloat::DecimalFloatErrors::abi_decode(output.as_ref()) {
return Err(FloatError::DecimalFloat(error));
}

Err(FloatError::Revert(output))
}
ExecutionResult::Halt { reason, .. } => Err(FloatError::Halt(reason)),
}
}
Loading