Migrate smart contract development environment from Hardhat to Foundry#225
Migrate smart contract development environment from Hardhat to Foundry#225Basharkhan7776 wants to merge 3 commits intoAOSSIE-Org:mainfrom
Conversation
…roduce Bun for client dependencies.
📝 WalkthroughWalkthroughMigrate the blockchain workspace from a Hardhat/JS setup to Foundry: replace tooling, Docker, and package config; remove Hardhat Ignition modules, JS tests, and artifacts; add Foundry config, remappings, Solidity tests, and refactor contracts and CCIP flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Caller
participant Factory as ElectionFactory
participant Generator as BallotGenerator / ElectionGenerator
participant Cloner as Clones
participant Election as Election Instance
participant Result as ResultCalculator
User->>Factory: createElection(electionInfo, candidates, ballotType, resultType)
Factory->>Generator: generateBallot(electionInfo, ballotType)
Factory->>Cloner: clone(electionGenerator)
Cloner-->>Factory: newElectionAddress
Factory->>Election: initialize(newElectionAddress, electionInfo, candidates, ballot, resultType)
Election-->>Factory: initialized
Factory-->>User: return electionAddress / update open list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@blockchain/.gitignore`:
- Line 6: The repo ignores lib/ but foundry.toml sets libs = ["lib",
"node_modules"] and remappings.txt expects dependencies under lib/, which breaks
builds for fresh clones; either remove lib from .gitignore and track the
dependency folders (or add them as git submodules) so remappings like
`@openzeppelin/contracts/`=lib/openzeppelin-contracts/contracts/ resolve, OR keep
lib ignored but add a clear README setup step instructing contributors to run
forge install after cloning (or provide a script) to fetch dependencies into
lib/ so remappings and builds succeed.
In `@blockchain/remappings.txt`:
- Line 5: The remapping entry "forge-std/=forge_std/src/" is invalid because the
directory doesn't exist; either remove this remapping from
blockchain/remappings.txt if forge-std isn't used, or correct it to the standard
Foundry path "forge-std/=lib/forge-std/src/" and ensure the dependency is added
to lib/ so the compiler can resolve it; update the remapping line and verify
compilation succeeds.
- Line 4: The remapping entry referencing ds-test points to a non-existent
nested path
(ds-test/=lib/openzeppelin-contracts/lib/forge-std/lib/ds-test/src/); either add
the dependency in that exact location or update the remapping to a valid ds-test
installation path. Fix by either (A) adding ds-test as a git submodule or
dependency so the directory lib/ds-test/src/ exists and change the remapping to
ds-test/=lib/ds-test/src/, or (B) if you installed ds-test elsewhere (e.g.,
lib/forge-std/lib/ds-test/src/ or node_modules/ds-test/src/), update the
remapping to that actual path; locate the entry in remappings.txt and replace
the invalid path with the correct one or remove it if ds-test is not used.
🧹 Nitpick comments (11)
blockchain/foundry.toml (2)
4-4: Consider removingnode_modulesfrom libs.Since this PR migrates away from npm/Hardhat, including
node_modulesin the libs array is inconsistent with the new tooling approach. If npm dependencies are truly removed, this entry is unnecessary and could cause confusion.-libs = ["node_modules", "lib"] +libs = ["lib"]
16-17: Missing etherscan configuration foramoynetwork.The
[rpc_endpoints]section defines bothsepoliaandamoy, but the[etherscan]section only configuressepolia. If contract verification on Polygon Amoy is needed, add the corresponding config:[etherscan] sepolia = { key = "${ETHERSCAN_KEY}" } +amoy = { key = "${POLYGONSCAN_KEY}" }blockchain/test/ElectionFactory.t.sol (2)
4-4: Remove unusedconsoleimport.The
consoleis imported but never used in the test file.-import {Test, console} from 'forge-std/Test.sol'; +import {Test} from 'forge-std/Test.sol';
98-110: Add end-to-end test to verify whitelist state changes.The test only confirms that owner calls don't revert, but doesn't verify the whitelist was actually modified. While the
approvedSenderContractsmapping lacks a public getter, you can test it indirectly through the_ccipReceivefunction, which validates incoming messages against the whitelist. Add a test that:
- Whitelists a contract for a chain
- Calls
_ccipReceivewith that sender (should succeed)- Removes the contract from the whitelist
- Calls
_ccipReceivewith that same sender (should revert withNotWhitelistedSender)This verifies that add/remove operations actually modify the whitelist state.
blockchain/README.md (3)
47-50: Use markdown link syntax for URLs.Per markdown best practices, wrap bare URLs in angle brackets or use proper link syntax.
-- Election Factory - https://sepolia.etherscan.io/address/0x64c720eBD5227bba57a5E8282893e71087cCcBb8#code +- Election Factory - <https://sepolia.etherscan.io/address/0x64c720eBD5227bba57a5E8282893e71087cCcBb8#code> - CCIP Sender :- - - Fuji - https://testnet.snowtrace.io//address/0xf267f0603672E791779E52B153BD9A39C9928767#code + - Fuji - <https://testnet.snowtrace.io//address/0xf267f0603672E791779E52B153BD9A39C9928767#code>
7-9: Consider adding dependency installation instructions.If
lib/is gitignored (as shown in.gitignore), users will need to runforge installafter cloning. Consider adding this to the prerequisites or as a setup step:### Prerequisites - [Foundry](https://book.getfoundry.sh/getting-started/installation) + +### Install Dependencies + +```shell +forge install +```
42-43: Clarify or update the deploy script example.The example references
script/MyScript.s.sol, which does not exist in the repository. Either update the path to an actual deployment script if one exists, or add a note explaining that users should create their own script following Foundry conventions.blockchain/Dockerfile (1)
1-1: Pin the Foundry image to a specific version for reproducible builds.Using
:latestis a moving tag that tracks the development branch and can cause unexpected build failures when the upstream image changes. Consider pinning to a stable release or an immutable tag:-FROM ghcr.io/foundry-rs/foundry:latest +FROM ghcr.io/foundry-rs/foundry:stableOr use an immutable nightly tag (e.g.,
nightly-<git_sha>) for more frequent updates while maintaining reproducibility.blockchain/test/Election.t.sol (3)
4-4: Unused import:consoleis never used.The
consoleimport fromforge-std/Test.solis not utilized in any test. Remove it to keep imports clean.Suggested fix
-import {Test, console} from 'forge-std/Test.sol'; +import {Test} from 'forge-std/Test.sol';
46-52: Minor typo in test fixture data.Line 50 has
'candidate2s'which appears to be a typo for'candidate2'. While this doesn't affect test correctness, fixing it improves consistency.Suggested fix
mockCandidates.push( Election.Candidate({ candidateID: 2, name: 'candidate2', - description: 'candidate2s' + description: 'candidate2' }) );
251-287: Minor style: Missing blank line before function.Add a blank line before
test_ResultCalculation_Schulze()for consistency with other test functions.Suggested fix
assertEq(winners[0], 1); } + function test_ResultCalculation_Schulze() public {
…nlink CCIP import paths.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@blockchain/contracts/ElectionFactory.sol`:
- Around line 59-95: createElection currently uses electionCount as the
election's stable ID but stores ownership keyed by the array index
(electionOwner[openBasedElections.length]), and deleteElection uses swap-and-pop
on openBasedElections which breaks the ID-to-index mapping; change ownership and
deletion to use the stable election ID instead: assign the ID (electionCount) to
the Election via Election.initialize, store ownership keyed by that ID
(electionOwner[electionId] = msg.sender), push the election address to
openBasedElections and also maintain a mapping electionIdToIndex (and optionally
indexToElectionId) that records the array index for each electionId, then update
deleteElection to look up the victim's array index via electionIdToIndex,
perform swap-and-pop on openBasedElections while updating both electionIdToIndex
entries and electionOwner by electionId (not by array index), and increment
electionCount after initialization; update all references to use electionId
mappings so swap-and-pop no longer corrupts ID correspondence.
🧹 Nitpick comments (3)
blockchain/package.json (2)
4-12: Replace the placeholder npm test script (and drop markdown in description).
npm testcurrently always fails, which can trip up default CI/dev workflows; wire it to your Foundry test command or remove it. Also npm descriptions are plain text, so the**Foundry**markdown will render literally.♻️ Proposed update
- "description": "This project contains the smart contracts for Agora Blockchain, built using **Foundry**.", + "description": "This project contains the smart contracts for Agora Blockchain, built using Foundry.",- "scripts": { - "test": "echo \"Error: no test specified\" && exit 1" - }, + "scripts": { + "test": "forge test" + },
17-20: Remove unusedlibreference from foundry.toml for clarity.The
lib/directory doesn't exist, and all remappings inremappings.txtconsistently point tonode_modules/. Consider removing"lib"from thelibsarray infoundry.toml(line 4) to avoid confusion about dependency sources.blockchain/contracts/mocks/MockCCIPReceiverRouter.sol (1)
37-40: Prefer selector constant over hard‑coded signature.Using
ElectionFactory.ccipReceive.selectoravoids signature drift if the struct layout changes.♻️ Proposed refactor
- bytes4 selector = bytes4( - keccak256('ccipReceive((bytes32,uint64,bytes,bytes,(address,uint256)[]))') - ); - bytes memory callData = abi.encodeWithSelector(selector, message); + bytes memory callData = abi.encodeWithSelector( + ElectionFactory.ccipReceive.selector, + message + );
…tory contract and its tests.
1 participant
Issue #149
Summary
This PR completely migrates the blockchain development environment from Hardhat to Foundry. It replaces the JavaScript-based testing and deployment workflow with a faster, Solidity-native Foundry setup. All existing tests have been ported to Solidity, and the project structure has been updated accordingly.
Changes
Configuration
npmdependencies with native Forge libraries (openzeppelin-contracts,chainlink-brownie-contracts) inlib/.hardhat.config.js, package.json,node_modules, and theignition/directory.Tests
ElectionFactory.test.js) to Solidity tests (.t.sol files) located intest/.DevOps
ghcr.io/foundry-rs/foundry:latestimage, optimizing the build container for Foundry.out,broadcast,cache_forge) correctly and ignore legacy Hardhat files.Documentation
forge build,forge test,anvil).Verification
forge testto execute the full test suite (15 passing tests).forge buildto verify contract compilation.Summary by CodeRabbit
Chores
Tests
Documentation
Contracts
✏️ Tip: You can customize this high-level summary in your review settings.