Rounding and billing fix#90
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
🟡 Heimdall Review Status
|
ilikesymmetry
left a comment
ilikesymmetry
left a comment
There was a problem hiding this comment.
Did not review tests, but looks like a relatively simple modification. Good principle learned: prefer custom net-amount args as calldata with math + checks against constraints versus passing in fields that derive the net-amount and restrict rounding preferences.
|
|
||
| // Pull tokens into this contract | ||
| IERC3009(token).receiveWithAuthorization({ | ||
| IERC3009(token) |
There was a problem hiding this comment.
are you on the latest version of foundry? wonder if there's a way to remove some of this lint thrashing... not blocking
| /// @param feeAmount Absolute fee in token units (must fall within payer-approved bounds) | ||
| /// @param feeReceiver Address to receive fees (should match the paymentInfo.feeReceiver unless that is 0 in which case it can be any address) | ||
| function capture(PaymentInfo calldata paymentInfo, uint256 amount, uint16 feeBps, address feeReceiver) | ||
| function capture(PaymentInfo calldata paymentInfo, uint256 amount, uint256 feeAmount, address feeReceiver) |
There was a problem hiding this comment.
one thing that came up in our original call was the question as to whether the new version of the AuthCaptureEscrow should retain its old interface too. I.e. should have have a net-new version of capture that offers the feeAmount while also offering a version with feeBps in case we want to migrate existing integrations to flow through the same AuthCaptureEscrow. A question for Fab and team. Given that we can run multiple authcaptureescrows in parallel, we would never be blocked on this, but might be good to consider one more time
4 participants
Replace feeBps with absolute feeAmount on capture() and charge(). Payer-approved minFeeBps/maxFeeBps still bound the fee as [amount × min / 10_000, amount × max / 10_000], so operators can pass cent-aligned fees off-chain without on-chain BPS truncation.
https://docs.google.com/document/d/1RCIXH0Q46XcwyYcu3027g9bKXHljrEX-y_ciBIdGNIQ/edit?tab=t.0#heading=h.onqsqfymwwr8
Implementation based on pps