Skip to content

Conversation

@pali101
Copy link
Contributor

@pali101 pali101 commented Jul 26, 2025

Summary

This PR introduces an auto-generated IPayments interface for the Payments contract, along with tooling and CI checks to ensure it remains consistent with the contract ABI.

Changes

  • Generated src/interfaces/IPayments.sol using abi-to-sol for external use and inheritance
  • Script: tools/generate-interface.sh builds the contract, extracts its ABI, and generates a clean IPayments interface
  • Refactor: Moved IValidator to src/interfaces/IValidator.sol
  • Makefile: Added make interface target
  • CI: GitHub Action to check interface drift on push/PR to main
  • Docs: Updated tools/README.md with usage details for the script and make interface

Closes #122

@rjan90 rjan90 moved this to 🔎 Awaiting review in Payments Jul 28, 2025
@rjan90 rjan90 added this to FS Jul 28, 2025
@github-project-automation github-project-automation bot moved this to 📌 Triage in FS Jul 28, 2025
@rjan90 rjan90 moved this from 📌 Triage to 🔎 Awaiting review in FS Jul 28, 2025
@rjan90 rjan90 requested a review from aarshkshah1992 July 28, 2025 08:25
@rjan90 rjan90 added this to the M2: Pandora Alpha on Mainnet milestone Jul 28, 2025

# Extract ABI using jq
echo "Generating interface at $OUTPUT_PATH..."
jq '.abi' out/Payments.sol/Payments.json | npx [email protected] IPayments --license MIT --solidity-version "^0.8.27" > "$OUTPUT_PATH"
Copy link
Contributor

Choose a reason for hiding this comment

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

optional, but you could pull these two versions up to variables at the top of the file so we can update it more easily, but it's not a big deal, we'll probably only ever change this when stuff doesn't work the way we expect -- unless we have the solidity version hard-wired somewhere else too?

@aarshkshah1992
Copy link
Contributor

@pali101 Let's keep this PR open till we integrate it with Pandora and make sure the interface/compilation/tests work in Pandora as well.

Copy link
Contributor

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

lgtm if it all works nicely locally (sorry I haven't had time to actually try it out myself)
I'll stop tormenting you now; good work!

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

rjan90 commented Jul 29, 2025

It seems like switching to interfaces did not have any impact on contract size, ref conversation that happenend in Slack here:

Switching to interface didn't have any effect on contract size. Unless we inherit it, it doesn't matter if we utilize contract or it's interface to call its functions.
Reference: https://ethereum.stackexchange.com/a/136871/67618

Do we expect that this PR will land, or should it be moved back to draft?

@rjan90
Copy link
Contributor

rjan90 commented Jul 29, 2025

From @aarshkshah1992 in Slack: It didn't get us the size reduction we needed. Now, there's no urgency here at all.

I will revert this back to draft for now, to signal that this is not expected to land until the audit is completed 2025-08-11

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

Labels

None yet

Projects

Status: ✔️ Approved by reviewer
Status: 🔎 Awaiting review

Development

Successfully merging this pull request may close these issues.

Interface for payments contract

4 participants