Skip to content

fix(gateway): replace transfer() with call() in withdrawFees#2238

Open
shlee-lab wants to merge 1 commit intokleros:devfrom
shlee-lab:fix/foreign-gateway-safe-send
Open

fix(gateway): replace transfer() with call() in withdrawFees#2238
shlee-lab wants to merge 1 commit intokleros:devfrom
shlee-lab:fix/foreign-gateway-safe-send

Conversation

@shlee-lab
Copy link
Copy Markdown
Contributor

@shlee-lab shlee-lab commented Feb 6, 2026

Summary

Replace .transfer() with .call{value:}() in ForeignGateway.withdrawFees()

Problem

.transfer() only forwards 2300 gas. If the relayer is a smart contract wallet (e.g., Gnosis Safe, automated relayer service), the fee withdrawal will fail.

Solution

Use .call{value:}() which forwards all available gas.

Changes

  • contracts/src/gateway/ForeignGateway.sol (3 lines)

PR-Codex overview

This PR focuses on enhancing the ForeignGateway contract by improving the payment transfer mechanism when resolving disputes, ensuring it handles failures more gracefully.

Detailed summary

  • Replaced direct transfer with a call method for transferring amount to dispute.relayer.
  • Added a check for the success of the transfer; if it fails, it reverts with a TransferFailed() error.
  • Introduced a new error TransferFailed() for handling transfer failures.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced fee withdrawal error handling with improved transfer mechanism and explicit failure notifications for unsuccessful withdrawal attempts.

Using .transfer() only forwards 2300 gas, which can cause failures
when the relayer is a smart contract wallet (e.g., Gnosis Safe).

Replace with .call{value:}() to forward all available gas.
@shlee-lab shlee-lab requested a review from jaybuidl as a code owner February 6, 2026 16:10
@netlify
Copy link
Copy Markdown

netlify bot commented Feb 6, 2026

‼️ Deploy request for kleros-v2-neo rejected.

Name Link
🔨 Latest commit b2faa6a

@netlify
Copy link
Copy Markdown

netlify bot commented Feb 6, 2026

👷 Deploy request for kleros-v2-testnet-devtools pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit b2faa6a

@netlify
Copy link
Copy Markdown

netlify bot commented Feb 6, 2026

👷 Deploy request for kleros-v2-testnet pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit b2faa6a

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 6, 2026

Walkthrough

The ForeignGateway contract's withdrawFees function now uses a low-level call instead of direct Ether transfer to send fees to relayers, with a corresponding new TransferFailed error for revert handling.

Changes

Cohort / File(s) Summary
Ether Transfer Mechanism
contracts/src/gateway/ForeignGateway.sol
Replaced direct Ether transfer with low-level call in withdrawFees function; added TransferFailed error for failure handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hops of approval with whiskers held high,
Low-level calls now catch fees that fly,
Transfer, revert, and fail with care,
SafeFees hopping everywhere!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: replacing transfer() with call() in withdrawFees, which is the primary objective of this pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@jaybuidl
Copy link
Copy Markdown
Member

jaybuidl commented Feb 6, 2026

Hey @shlee-lab thanks for the contribution. For ETH transfers we have been using another pattern which is to rely on the SafeSend.sol library. The gateways are lagging a bit behind and need to start using SafeSend too.

@shlee-lab
Copy link
Copy Markdown
Contributor Author

@jaybuidl Thanks for the guidance! I'll update this PR to use the SafeSend.sol library instead of raw .call(). One questio, does the ForeignGateway already have a WETH address stored, or should I add a new state variable / initializer parameter for it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants