feat: include feeAmount in batch transaction value for ETH_FEE_PROXY_CONTRACT payments #1674

aimensahnoun · 2025-12-18T12:04:22Z

Description of the changes

Fixed a bug in the batch conversion proxy where fee amounts were not included in the transaction value for ETH_FEE_PROXY_CONTRACT payments. The code now properly adds the fee amount to the total transaction value, ensuring that the correct amount is sent when processing payments. Added comprehensive tests to verify the fix works for both single payments and batches of payments with fees.

Summary by CodeRabbit

Bug Fixes
- Batch payment processing now correctly includes fee amounts in transaction value calculations for Ethereum fee proxy contracts, ensuring accurate fee accounting in multi-transaction batches.
Tests
- Added validation tests confirming fee amounts are properly included in batch payment transaction calculations across single and multiple payment scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…CONTRACT payments

coderabbitai · 2025-12-18T12:04:31Z

Walkthrough

Logic in getBatchTxValue is refactored to handle three payment network types distinctly: ANY_TO_ETH_PROXY adds maxToSpend, ETH_FEE_PROXY_CONTRACT adds base amount plus feeAmount, and others retain previous skip behavior. Tests validate feeAmount inclusion in batch transaction values for ETH_FEE_PROXY_CONTRACT payments.

Changes

Cohort / File(s)	Summary
ETH Fee Proxy Contract Support `packages/payment-processor/src/payment/batch-conversion-proxy.ts`	Modified `getBatchTxValue` function to branch by paymentNetworkId: handles ANY_TO_ETH_PROXY, ETH_FEE_PROXY_CONTRACT (including feeAmount in value calculation), and other networks with explicit logic paths replacing previous single conditional.
Fee Handling Tests `packages/payment-processor/test/payment/batch-proxy.test.ts`	Added two test cases validating feeAmount inclusion in batch conversion values: single ETH_FEE_PROXY_CONTRACT payment test and dual-payment batch test; asserts computed value differs when fees are included.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Payment network type branching logic is straightforward but requires verification that feeAmount is correctly attributed
Test cases are clear and direct assertions
Verify previous behavior for non-ETH_FEE_PROXY_CONTRACT networks is preserved

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: including feeAmount in batch transaction values for ETH_FEE_PROXY_CONTRACT payments, which matches the core purpose of the changeset.
Description check	✅ Passed	The pull request description includes the required section 'Description of the changes' and provides clear details about the bug fix, the solution, and the test coverage added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch 12-18-feat_include_feeamount_in_batch_transaction_value_for_eth_fee_proxy_contract_payments

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 817bff3 and 5b172de.

📒 Files selected for processing (2)

packages/payment-processor/src/payment/batch-conversion-proxy.ts (1 hunks)
packages/payment-processor/test/payment/batch-proxy.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*

⚙️ CodeRabbit configuration file

**/*: - Only comment on issues that would block merging — ignore minor or stylistic concerns.

Restrict feedback to errors, security risks, or functionality-breaking problems.

Do not post comments on code style, formatting, or non-critical improvements.

Keep reviews short: flag only issues that make the PR unsafe to merge.

Limit review comments to 3–5 items maximum, unless additional blockers exist.

Group similar issues into a single comment instead of posting multiple notes.

Skip repetition — if a pattern repeats, mention it once at a summary level only.

Do not add general suggestions; focus strictly on merge-blocking concerns.

If there are no critical problems, respond with minimal approval (e.g., 'Looks good'). Do not add additional review.

Avoid line-by-line commentary unless it highlights a critical bug or security hole.

Highlight only issues that could cause runtime errors, data loss, or severe maintainability issues.

Ignore minor optimization opportunities — focus solely on correctness and safety.

Provide a top-level summary of critical blockers rather than detailed per-line notes.

Comment only when the issue must be resolved before merge — otherwise, remain silent.

When in doubt, err on the side of fewer comments — brevity and blocking issues only.

Avoid posting any refactoring issues

Files:

packages/payment-processor/src/payment/batch-conversion-proxy.ts
packages/payment-processor/test/payment/batch-proxy.test.ts

🧠 Learnings (19)

📓 Common learnings

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1453
File: packages/smart-contracts/src/contracts/ERC20SingleRequestProxy.sol:46-46
Timestamp: 2024-10-05T14:43:16.298Z
Learning: In `packages/smart-contracts/src/contracts/ERC20SingleRequestProxy.sol`, within the `receive()` function, it's acceptable to approve the entire `balance` to `erc20FeeProxy` because `balance == paymentAmount + feeAmount`.

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:237-246
Timestamp: 2024-11-08T18:24:06.144Z
Learning: In `packages/payment-processor/test/payment/single-request-proxy.test.ts`, when asserting the `feeProxyUsed` in emitted events from `SingleRequestProxyFactory`, retrieve the `erc20FeeProxy` (or `ethereumFeeProxy`) public variable from the `SingleRequestProxyFactory` contract instead of using `wallet.address`.

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1453
File: packages/smart-contracts/test/contracts/ERC20SingleRequestProxy.test.ts:150-152
Timestamp: 2024-10-17T18:30:55.410Z
Learning: In `packages/smart-contracts/test/contracts/ERC20SingleRequestProxy.test.ts`, the skipped test `'should process a partial payment correctly'` exists intentionally to show that partial payments are supported without duplicating previous happy-path tests.

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:198-206
Timestamp: 2024-11-08T18:24:19.095Z
Learning: In tests for `SingleRequestProxyFactory`, the `feeProxyUsed` in events should be verified by retrieving the `ethereumFeeProxy` public variable from the `SingleRequestProxyFactory` contract, not `wallet.address`.

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:251-270
Timestamp: 2024-10-28T16:03:33.215Z
Learning: When testing the payment-processor module, specifically in `packages/payment-processor/test/payment/single-request-proxy.test.ts`, it's acceptable to omit tests for partial payments if they have already been covered at the smart-contract level.

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1484
File: packages/advanced-logic/test/extensions/payment-network/any-to-near.test.ts:0-0
Timestamp: 2024-11-06T14:48:18.698Z
Learning: In `packages/advanced-logic/test/extensions/payment-network/any-to-near.test.ts`, when the existing happy path tests are deemed sufficient, additional test cases may not be necessary.

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1453
File: packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol:0-0
Timestamp: 2024-09-27T11:42:01.062Z
Learning: In the `createEthereumSingleRequestProxy` function, it's acceptable for `_feeAddress` to be the zero address, as setting the fee address to zero is a common practice. Adding zero address checks is unnecessary and adds extra gas cost.

Learnt from: giorgi-kiknavelidze
Repo: RequestNetwork/requestNetwork PR: 1482
File: packages/usage-examples/src/hinkal/testPayErc20FeeProxyRequestHinkal.ts:19-27
Timestamp: 2024-11-04T12:18:13.888Z
Learning: In the file `packages/usage-examples/src/hinkal/testPayErc20FeeProxyRequestHinkal.ts`, when using `payPrivateErc20FeeProxyRequest`, the returned `tx` is a `RelayerTransaction`, so calling `.wait()` on it is unnecessary.

Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1488
File: packages/payment-processor/src/payment/single-request-forwarder.ts:211-219
Timestamp: 2024-11-12T17:48:47.072Z
Learning: In `packages/payment-processor/src/payment/single-request-forwarder.ts`, the error handling logic in `payWithEthereumSingleRequestForwarder` is correct and does not require changes to differentiate between ERC20 and Ethereum SingleRequestForwarder contracts.

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1481
File: packages/integration-test/test/scheduled/erc20-proxy.test.ts:0-0
Timestamp: 2024-11-05T16:53:05.280Z
Learning: In `packages/integration-test/test/scheduled/erc20-proxy.test.ts`, when upgrading dependencies like `ethers`, additional error handling test cases for contract interactions and provider errors may not be necessary.

📚 Learning: 2024-11-08T18:24:06.144Z

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:237-246
Timestamp: 2024-11-08T18:24:06.144Z
Learning: In `packages/payment-processor/test/payment/single-request-proxy.test.ts`, when asserting the `feeProxyUsed` in emitted events from `SingleRequestProxyFactory`, retrieve the `erc20FeeProxy` (or `ethereumFeeProxy`) public variable from the `SingleRequestProxyFactory` contract instead of using `wallet.address`.

Applied to files:

packages/payment-processor/src/payment/batch-conversion-proxy.ts
packages/payment-processor/test/payment/batch-proxy.test.ts

📚 Learning: 2024-11-12T16:54:02.702Z

Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1488
File: packages/payment-processor/src/payment/single-request-forwarder.ts:104-104
Timestamp: 2024-11-12T16:54:02.702Z
Learning: In `packages/payment-processor/src/payment/single-request-forwarder.ts`, when the smart contract has not changed, event argument names such as `proxyAddress` remain the same, even if variable names in the code are updated to use new terminology like `forwarderAddress`.

Applied to files:

packages/payment-processor/src/payment/batch-conversion-proxy.ts

📚 Learning: 2024-10-29T09:00:54.169Z

Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/payment-processor/src/payment/single-request-proxy.ts:202-209
Timestamp: 2024-10-29T09:00:54.169Z
Learning: In the `packages/payment-processor/src/payment/single-request-proxy.ts` file, within the `payWithEthereumSingleRequestProxy` function, the current error handling is acceptable as per the project's expectations.

Applied to files:

packages/payment-processor/src/payment/batch-conversion-proxy.ts
packages/payment-processor/test/payment/batch-proxy.test.ts

📚 Learning: 2024-11-12T17:48:47.072Z

Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1488
File: packages/payment-processor/src/payment/single-request-forwarder.ts:211-219
Timestamp: 2024-11-12T17:48:47.072Z
Learning: In `packages/payment-processor/src/payment/single-request-forwarder.ts`, the error handling logic in `payWithEthereumSingleRequestForwarder` is correct and does not require changes to differentiate between ERC20 and Ethereum SingleRequestForwarder contracts.

Applied to files:

packages/payment-processor/src/payment/batch-conversion-proxy.ts
packages/payment-processor/test/payment/batch-proxy.test.ts

📚 Learning: 2024-11-04T14:32:57.040Z

Learnt from: giorgi-kiknavelidze
Repo: RequestNetwork/requestNetwork PR: 1482
File: packages/payment-processor/src/payment/erc20-fee-proxy.ts:186-186
Timestamp: 2024-11-04T14:32:57.040Z
Learning: In the `preparePrivateErc20FeeProxyPaymentTransaction` function in `packages/payment-processor/src/payment/erc20-fee-proxy.ts`, we should not make an allowance call on the token contract, as it is not relevant to our use case.

Applied to files:

packages/payment-processor/src/payment/batch-conversion-proxy.ts

📚 Learning: 2024-10-28T16:03:33.215Z

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:251-270
Timestamp: 2024-10-28T16:03:33.215Z
Learning: When testing the payment-processor module, specifically in `packages/payment-processor/test/payment/single-request-proxy.test.ts`, it's acceptable to omit tests for partial payments if they have already been covered at the smart-contract level.

Applied to files:

packages/payment-processor/src/payment/batch-conversion-proxy.ts
packages/payment-processor/test/payment/batch-proxy.test.ts

📚 Learning: 2024-11-08T20:01:10.313Z

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/src/payment/single-request-proxy.ts:0-0
Timestamp: 2024-11-08T20:01:10.313Z
Learning: In `packages/payment-processor/src/payment/single-request-proxy.ts`, the team prefers not to store or validate decoded values that are not needed.

Applied to files:

packages/payment-processor/src/payment/batch-conversion-proxy.ts

📚 Learning: 2024-10-17T18:30:55.410Z

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1453
File: packages/smart-contracts/test/contracts/ERC20SingleRequestProxy.test.ts:150-152
Timestamp: 2024-10-17T18:30:55.410Z
Learning: In `packages/smart-contracts/test/contracts/ERC20SingleRequestProxy.test.ts`, the skipped test `'should process a partial payment correctly'` exists intentionally to show that partial payments are supported without duplicating previous happy-path tests.

Applied to files:

packages/payment-processor/src/payment/batch-conversion-proxy.ts
packages/payment-processor/test/payment/batch-proxy.test.ts

📚 Learning: 2024-11-08T20:01:10.313Z

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/src/payment/single-request-proxy.ts:0-0
Timestamp: 2024-11-08T20:01:10.313Z
Learning: In `packages/payment-processor/src/payment/single-request-proxy.ts`, when decoding event data, the team prefers not to include documentation of the event data structure if maintaining it would be difficult.

Applied to files:

packages/payment-processor/src/payment/batch-conversion-proxy.ts

📚 Learning: 2024-12-09T18:59:04.613Z

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1482
File: packages/payment-processor/src/payment/erc-20-private-payment-hinkal.ts:80-99
Timestamp: 2024-12-09T18:59:04.613Z
Learning: In the `RequestNetwork` codebase, payment processor functions such as `payErc20ProxyRequestFromHinkalShieldedAddress` in `packages/payment-processor/src/payment/erc-20-private-payment-hinkal.ts` should not include explicit request validation or try/catch blocks, and should rely on the underlying components (like `hinkalObject`) to handle error reporting.

Applied to files:

packages/payment-processor/src/payment/batch-conversion-proxy.ts

📚 Learning: 2024-10-05T14:43:16.298Z

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1453
File: packages/smart-contracts/src/contracts/ERC20SingleRequestProxy.sol:46-46
Timestamp: 2024-10-05T14:43:16.298Z
Learning: In `packages/smart-contracts/src/contracts/ERC20SingleRequestProxy.sol`, within the `receive()` function, it's acceptable to approve the entire `balance` to `erc20FeeProxy` because `balance == paymentAmount + feeAmount`.

Applied to files:

packages/payment-processor/src/payment/batch-conversion-proxy.ts

📚 Learning: 2024-11-05T16:53:05.280Z

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1481
File: packages/integration-test/test/scheduled/erc20-proxy.test.ts:0-0
Timestamp: 2024-11-05T16:53:05.280Z
Learning: In `packages/integration-test/test/scheduled/erc20-proxy.test.ts`, when upgrading dependencies like `ethers`, additional error handling test cases for contract interactions and provider errors may not be necessary.

Applied to files:

packages/payment-processor/test/payment/batch-proxy.test.ts

📚 Learning: 2024-11-08T18:24:19.095Z

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1487
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:198-206
Timestamp: 2024-11-08T18:24:19.095Z
Learning: In tests for `SingleRequestProxyFactory`, the `feeProxyUsed` in events should be verified by retrieving the `ethereumFeeProxy` public variable from the `SingleRequestProxyFactory` contract, not `wallet.address`.

Applied to files:

packages/payment-processor/test/payment/batch-proxy.test.ts

📚 Learning: 2024-10-29T08:03:10.463Z

Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:225-228
Timestamp: 2024-10-29T08:03:10.463Z
Learning: In 'packages/payment-processor/test/payment/single-request-proxy.test.ts' (TypeScript), when testing with an invalid proxy address, we should not use the zero address.

Applied to files:

packages/payment-processor/test/payment/batch-proxy.test.ts

📚 Learning: 2024-10-29T08:02:02.600Z

Learnt from: aimensahnoun
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:138-139
Timestamp: 2024-10-29T08:02:02.600Z
Learning: When testing invalid requests in `packages/payment-processor/test/payment/single-request-proxy.test.ts`, it's acceptable to use `ts-expect-error` to suppress TypeScript errors when the request intentionally lacks required properties.

Applied to files:

packages/payment-processor/test/payment/batch-proxy.test.ts

📚 Learning: 2024-11-04T12:18:13.888Z

Learnt from: giorgi-kiknavelidze
Repo: RequestNetwork/requestNetwork PR: 1482
File: packages/usage-examples/src/hinkal/testPayErc20FeeProxyRequestHinkal.ts:19-27
Timestamp: 2024-11-04T12:18:13.888Z
Learning: In the file `packages/usage-examples/src/hinkal/testPayErc20FeeProxyRequestHinkal.ts`, when using `payPrivateErc20FeeProxyRequest`, the returned `tx` is a `RelayerTransaction`, so calling `.wait()` on it is unnecessary.

Applied to files:

packages/payment-processor/test/payment/batch-proxy.test.ts

📚 Learning: 2024-11-06T14:48:18.698Z

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1484
File: packages/advanced-logic/test/extensions/payment-network/any-to-near.test.ts:0-0
Timestamp: 2024-11-06T14:48:18.698Z
Learning: In `packages/advanced-logic/test/extensions/payment-network/any-to-near.test.ts`, when the existing happy path tests are deemed sufficient, additional test cases may not be necessary.

Applied to files:

packages/payment-processor/test/payment/batch-proxy.test.ts

📚 Learning: 2024-10-28T20:00:33.707Z

Learnt from: MantisClone
Repo: RequestNetwork/requestNetwork PR: 1474
File: packages/smart-contracts/scripts/test-deploy-single-request-proxy.ts:4-7
Timestamp: 2024-10-28T20:00:33.707Z
Learning: In TypeScript test files (e.g., `test-deploy-single-request-proxy.ts`), it's acceptable to use `string` types for Ethereum addresses in interfaces (like `FeeProxyAddresses`), as stricter type safety is not necessary.

Applied to files:

packages/payment-processor/test/payment/batch-proxy.test.ts

🧬 Code graph analysis (1)

packages/payment-processor/test/payment/batch-proxy.test.ts (2)

packages/payment-processor/src/types.ts (1)

EnrichedRequest (89-95)

packages/payment-processor/src/payment/batch-conversion-proxy.ts (1)

prepareBatchConversionPaymentTransaction (82-98)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: Greptile Review
GitHub Check: build-and-test

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

aimensahnoun · 2025-12-18T12:04:39Z

feat: include feeAmount in batch transaction value for ETH_FEE_PROXY_CONTRACT payments #1674 👈 (View in Graphite)
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

bassgeta

LGTM 🚢

rodrigopavezi

good one

feat: include feeAmount in batch transaction value for ETH_FEE_PROXY_…

5b172de

…CONTRACT payments

aimensahnoun requested review from MantisClone, bassgeta and rodrigopavezi December 18, 2025 12:04

aimensahnoun self-assigned this Dec 18, 2025

aimensahnoun added the bug label Dec 18, 2025 — with Graphite App

aimensahnoun marked this pull request as ready for review December 18, 2025 12:05

github-actions bot requested review from alexandre-abrioux-rf and yomarion-rf-admin December 18, 2025 12:05

bassgeta approved these changes Dec 18, 2025

View reviewed changes

rodrigopavezi approved these changes Dec 18, 2025

View reviewed changes

aimensahnoun merged commit 2eb3fcb into master Dec 18, 2025
21 checks passed

aimensahnoun deleted the 12-18-feat_include_feeamount_in_batch_transaction_value_for_eth_fee_proxy_contract_payments branch December 18, 2025 13:53

aimensahnoun linked an issue Dec 18, 2025 that may be closed by this pull request

getBatchTxValue() doesn't include feeAmount for ETH_FEE_PROXY_CONTRACT, causing "Not enough funds" error #1672

Closed

MantisClone mentioned this pull request Dec 18, 2025

chore: publish 0.61.0 #1675

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: include feeAmount in batch transaction value for ETH_FEE_PROXY_CONTRACT payments #1674

feat: include feeAmount in batch transaction value for ETH_FEE_PROXY_CONTRACT payments #1674

Uh oh!

aimensahnoun commented Dec 18, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 18, 2025 •

edited

Loading

Uh oh!

aimensahnoun commented Dec 18, 2025

Uh oh!

bassgeta left a comment

Uh oh!

rodrigopavezi left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feat: include feeAmount in batch transaction value for ETH_FEE_PROXY_CONTRACT payments #1674

feat: include feeAmount in batch transaction value for ETH_FEE_PROXY_CONTRACT payments #1674

Uh oh!

Conversation

aimensahnoun commented Dec 18, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the changes

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

aimensahnoun commented Dec 18, 2025

Uh oh!

bassgeta left a comment

Choose a reason for hiding this comment

Uh oh!

rodrigopavezi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aimensahnoun commented Dec 18, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 18, 2025 •

edited

Loading