Skip to content

Conversation

@jmg-duarte
Copy link
Contributor

Description

Migrates the remaining_amounts module to alloy.

Note: a big chunk of the diff was just moving the Remaining impl closer to the struct, the diff is just eh at noticing.

Changes

  • Refactors the "ratio" module into a trait and moves it out of shared to the more adequate number crate
  • Adds a type alias to make it easier to use the new RatioExt in the module
  • Refactors the call sites

How to test

Existing tests

@jmg-duarte jmg-duarte requested a review from a team as a code owner December 11, 2025 12:08
@jmg-duarte jmg-duarte marked this pull request as draft December 11, 2025 13:25
Base automatically changed from jmgd/alloy/nonzero to main December 11, 2025 14:30
@jmg-duarte jmg-duarte changed the base branch from main to jmgd/alloy/eth_trait December 11, 2025 15:13
@jmg-duarte jmg-duarte marked this pull request as ready for review December 11, 2025 16:50
Comment on lines +14 to +17
/// Multiplies a ratio by a scalar, returning `None` only if the result
/// would overflow a `U256`, but intermediate operations are allowed to
/// overflow.
fn full_scalar_mul(&self, scalar: N) -> Option<N>;
Copy link
Contributor

Choose a reason for hiding this comment

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

With alloy's number types being so efficient I wonder if this shouldn't just be the default behavior.
Usually the only reason why one would want intermediate overflows to cause errors is when you try to model the behavior of a smart contract that does that. This is not the case here.

Fine to merge without addressing this since this is a faithful conversion of the existing code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default behaviour for what though?

Base automatically changed from jmgd/alloy/eth_trait to main December 12, 2025 10:14
@jmg-duarte jmg-duarte enabled auto-merge December 12, 2025 10:23
@jmg-duarte jmg-duarte added this pull request to the merge queue Dec 12, 2025
Merged via the queue into main with commit 42048ea Dec 12, 2025
18 checks passed
@jmg-duarte jmg-duarte deleted the jmgd/alloy/remaining branch December 12, 2025 10:46
@github-actions github-actions bot locked and limited conversation to collaborators Dec 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants