Skip to content

Conversation

@pali101
Copy link
Contributor

@pali101 pali101 commented Aug 5, 2025

Summary

This PR adds a GitHub Actions workflow that checks if smart contract ABIs in subgraph/abis/ match the Solidity source. It regenerates ABIs using make abi and fails if differences with committed files are found, ensuring ABI updates accompany contract changes and preventing integration issues.

Changes

  • added a new GitHub Actions workflow Check ABI consistency
  • added a make target abi that generates ABIs for all contracts in service_contracts/src (excluding libraries)
  • added a script service_contracts/tools/generate-abi.sh used both in CI and the Makefile for ABI generation

@rjan90 rjan90 added this to FS Aug 5, 2025
@github-project-automation github-project-automation bot moved this to 📌 Triage in FS Aug 5, 2025
@rjan90 rjan90 moved this from 📌 Triage to 🔎 Awaiting review in FS Aug 5, 2025
@rjan90 rjan90 linked an issue Aug 5, 2025 that may be closed by this pull request
3 tasks
@rjan90 rjan90 added this to the MX: Priority and sequencing TBD milestone Aug 5, 2025
@rjan90 rjan90 requested a review from rvagg August 5, 2025 11:51
@ZenGround0
Copy link
Collaborator

@hugomrdias I think this will resolve your issue -- at least with this contract.

@jennijuju jennijuju requested a review from Copilot August 15, 2025 11:20
Copy link
Contributor

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 implements a CI workflow to ensure ABI files in the subgraph stay synchronized with smart contract source code. The workflow automatically regenerates ABIs and fails if they differ from committed files, preventing integration issues.

  • Added a GitHub Actions workflow that verifies ABI consistency on push/PR to main
  • Created a new make abi target that generates ABIs for all contracts (excluding libraries)
  • Added a bash script to extract ABIs from Solidity contracts using forge and jq

Reviewed Changes

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

File Description
.github/workflows/check-abi-consistency.yml New CI workflow that checks ABI consistency by regenerating ABIs and comparing with committed files
service_contracts/Makefile Added abi target to generate ABI files for all contracts in src/ directory
service_contracts/tools/generate-abi.sh New script that extracts ABIs from compiled contracts using forge and jq
subgraph/abis/FilecoinWarmStorageService.json Updated ABI file reflecting changes to the FilecoinWarmStorageService contract

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

# Extract ABI for each contract in the source directory
for contract in "${contracts[@]}"; do
mkdir -p ${OUTPUT_DIR}/
jq '.abi' "${SRC_DIR}/../out/${contract}.sol/${contract}.json" > "${OUTPUT_DIR}/${contract}.json"
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

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

The path construction assumes the contract filename matches the contract name exactly (${contract}.sol), but Solidity files can contain multiple contracts or have different naming conventions. This could fail if the contract name doesn't match the file name.

Suggested change
jq '.abi' "${SRC_DIR}/../out/${contract}.sol/${contract}.json" > "${OUTPUT_DIR}/${contract}.json"
mkdir -p "${OUTPUT_DIR}/"
abi_file=$(find "${SRC_DIR}/../out" -type f -name "${contract}.json" | head -n 1)
if [[ -z "$abi_file" ]]; then
echo "ABI file for contract $contract not found in ${SRC_DIR}/../out"
continue
fi
jq '.abi' "$abi_file" > "${OUTPUT_DIR}/${contract}.json"

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@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.

love it

@github-project-automation github-project-automation bot moved this from 🔎 Awaiting review to ✔️ Approved by reviewer in FS Aug 18, 2025
@rvagg
Copy link
Collaborator

rvagg commented Aug 18, 2025

needs merge conflict resolution though -- I think if I were you I'd rebase, re-gen and force push over the top of whatever the conflict is in that .json, up to you how you handle it as long as it results in a correct .json

@pali101 pali101 force-pushed the chore/abi-generation-ci branch from d837190 to 2d50788 Compare August 19, 2025 06:26
@rjan90
Copy link
Collaborator

rjan90 commented Sep 22, 2025

Hey @pali101! 👋 I'm not sure why this never got merged before conflicts arose again. I think if I read the code and history correct, it looks like the ABI consistency workflow was ultimately implemented in PR #180 which added the check-abi

job. PR #200 and #202 then added the additional specific ABI files.

Even though this PR wasn't the one that got merged, thank you so much for your contribution @pali101! 🙏 Your work here clearly helped shape the thinking around ABI consistency, I can see similar patterns and approaches in what ultimately got implemented in PR #180.

Will make sure this gets reflected in the PLDG impact rating, your contribution was valuable even if the timing and merge conflicts didn't work out perfectly. CC: @rvagg if there is anything else from this PR that we want to salvage before closing it out.

@rjan90 rjan90 closed this Sep 24, 2025
@github-project-automation github-project-automation bot moved this from ✔️ Approved by reviewer to 🎉 Done in FS Sep 24, 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

Development

Successfully merging this pull request may close these issues.

Add and check contract ABIs

4 participants