Skip to content

Conversation

pali101
Copy link
Contributor

@pali101 pali101 commented Aug 20, 2025

Summary

Adds ERC-3009 deposit flows, event extensions, and tests.

Changes

  • Introduce IERC3009.
  • Payments
    • Add AuthType enum; extend DepositRecorded(token, from, to, amount, authType, nonce).
    • Generalize self-recipient check: PermitRecipientMustBeMsgSenderSignerMustBeMsgSender; validatePermitRecipientvalidateSignerIsRecipient.
    • New methods: depositWithAuthorization(...), depositWithAuthorizationAndApproveOperator(...), depositWithAuthorizationAndIncreaseOperatorApproval(...)
  • Tests
    • Comprehensive coverage for happy path, replay, signature errors, time windows, sender mismatch, insufficient balance, domain mismatch.
  • Mock
    • MockERC20: implements ERC-3009.

Notes

  • authorizationState used for replay protection
  • domain separation enforced by signatures.

Closes #205

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for ERC-3009 deposit flows to the Payments contract, enabling meta-transactions and gasless deposits using cryptographic signatures. ERC-3009 provides an alternative to ERC-2612 permits by allowing transfers to be authorized via signatures without requiring on-chain approvals.

  • Introduces IERC3009 interface and three new deposit methods leveraging authorization signatures
  • Generalizes signature-based validation by renaming permit-specific error and modifier names
  • Extends the DepositRecorded event to include authorization type and nonce for better tracking

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/mocks/MockERC20.sol Implements ERC-3009 functionality with transferWithAuthorization and receiveWithAuthorization methods
test/helpers/PaymentsTestHelpers.sol Adds helper functions for ERC-3009 signature generation and test scenarios
test/PaymentsEvents.t.sol Updates event emission tests to include new AuthType and nonce parameters
test/DepositWithAuthorizationAndOperatorApproval.t.sol Comprehensive test coverage for ERC-3009 deposit flows with operator approval
test/DepositWithAuthorization.t.sol Test coverage for basic ERC-3009 deposit functionality
src/Payments.sol Core implementation of ERC-3009 deposit methods and interface definition
src/Errors.sol Generalizes error naming from permit-specific to signature-based validation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@BigLep BigLep added this to FS Aug 20, 2025
@github-project-automation github-project-automation bot moved this to 📌 Triage in FS Aug 20, 2025
@BigLep BigLep moved this to 🔎 Awaiting review in Payments Aug 20, 2025
@rjan90 rjan90 moved this from 📌 Triage to 🔎 Awaiting review in FS Aug 20, 2025
@github-project-automation github-project-automation bot moved this from 🔎 Awaiting review to ⌨️ In Progress in FS Aug 20, 2025
@rjan90 rjan90 moved this from ⌨️ In Progress to 🔎 Awaiting review in FS Aug 21, 2025
rjan90 pushed a commit that referenced this pull request Aug 21, 2025
Adding pull_request_target so PRs like
#214 get automatically
added to the board.
Copy link

codecov bot commented Sep 3, 2025

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/Payments.sol 91.66% 2 Missing ⚠️

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #214   +/-   ##
=======================================
  Coverage        ?   81.25%           
=======================================
  Files           ?        2           
  Lines           ?      480           
  Branches        ?      100           
=======================================
  Hits            ?      390           
  Misses          ?       84           
  Partials        ?        6           
Flag Coverage Δ
foundry 81.25% <91.66%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/Payments.sol 81.87% <91.66%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pali101 pali101 requested a review from wjmelements September 4, 2025 20:10
src/Payments.sol Outdated
* @custom:constraint Operator must already be approved.
*/
function depositWithAuthorizationAndIncreaseOperatorApproval(
address token,
Copy link
Collaborator

Choose a reason for hiding this comment

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

IERC3009 token

pali101 pushed a commit to pali101/fws-payments that referenced this pull request Sep 5, 2025
Adding pull_request_target so PRs like
FilOzone#214 get automatically
added to the board.
@wjmelements
Copy link
Collaborator

I wonder why your merge looks like this

Screenshot 2025-09-05 at 2 58 56 PM

Normally Github only shows the merge commit, not all of the commits in the other branch. Example How did you merge main in git?

Screenshot 2025-09-05 at 3 00 13 PM

@github-project-automation github-project-automation bot moved this from 🔎 Awaiting review to ✔️ Approved by reviewer in FS Sep 8, 2025
@rjan90
Copy link
Contributor

rjan90 commented Sep 10, 2025

@pali101 It seems like there is only three small nits that you need to address here, before we can merge this PR 🙏

@wjmelements wjmelements merged commit 5a34645 into FilOzone:main Sep 12, 2025
6 checks passed
@github-project-automation github-project-automation bot moved this from 🔎 Awaiting review to 🎉 Done in Payments Sep 12, 2025
@github-project-automation github-project-automation bot moved this from ✔️ Approved by reviewer to 🎉 Done in FS Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🎉 Done
Status: 🎉 Done
Development

Successfully merging this pull request may close these issues.

Add ERC-3009 Support
3 participants