Expose the Pausable ABI as an interface#6559
Conversation
🦋 Changeset detectedLatest commit: b0f32b5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR extracts pause-related events and errors from the 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/utils/IPausable.test.js`:
- Around line 8-16: The test currently uses inclusive assertions which allow
extra ABI entries; update the assertions in IPausable.test.js to assert exact
membership and count by comparing the sorted/unique names array (variable names
from the ABI: abi -> names) to the exact expected list
['Paused','Unpaused','EnforcedPause','ExpectedPause','paused'] and assert the
length matches that list (or use deep equality on sets/arrays) and remove the
includes/does-not-include checks (replace expect(names).to.include/... and
expect(...).to.not.include/... with a single exact equality/length assertion
against the expected entries).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d8a80f1f-1a16-40c3-8d56-9f705c0d915c
📒 Files selected for processing (4)
.changeset/wet-mirrors-invent.mdcontracts/utils/IPausable.solcontracts/utils/Pausable.soltest/utils/IPausable.test.js
| const names = abi.map(fragment => fragment.name).filter(Boolean); | ||
|
|
||
| expect(names).to.include('Paused'); | ||
| expect(names).to.include('Unpaused'); | ||
| expect(names).to.include('EnforcedPause'); | ||
| expect(names).to.include('ExpectedPause'); | ||
| expect(names).to.include('paused'); | ||
| expect(names).to.not.include('pause'); | ||
| expect(names).to.not.include('unpause'); |
There was a problem hiding this comment.
Make the ABI assertion exact, not just inclusive.
This test currently allows unexpected extra ABI entries. To match the “only expected entries” objective, assert exact membership (and length).
Suggested tightening
- const names = abi.map(fragment => fragment.name).filter(Boolean);
-
- expect(names).to.include('Paused');
- expect(names).to.include('Unpaused');
- expect(names).to.include('EnforcedPause');
- expect(names).to.include('ExpectedPause');
- expect(names).to.include('paused');
- expect(names).to.not.include('pause');
- expect(names).to.not.include('unpause');
+ const names = abi.map(fragment => fragment.name).filter(Boolean).sort();
+ expect(names).to.deep.equal([
+ 'EnforcedPause',
+ 'ExpectedPause',
+ 'Paused',
+ 'Unpaused',
+ 'paused',
+ ].sort());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const names = abi.map(fragment => fragment.name).filter(Boolean); | |
| expect(names).to.include('Paused'); | |
| expect(names).to.include('Unpaused'); | |
| expect(names).to.include('EnforcedPause'); | |
| expect(names).to.include('ExpectedPause'); | |
| expect(names).to.include('paused'); | |
| expect(names).to.not.include('pause'); | |
| expect(names).to.not.include('unpause'); | |
| const names = abi.map(fragment => fragment.name).filter(Boolean).sort(); | |
| expect(names).to.deep.equal([ | |
| 'EnforcedPause', | |
| 'ExpectedPause', | |
| 'Paused', | |
| 'Unpaused', | |
| 'paused', | |
| ].sort()); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/utils/IPausable.test.js` around lines 8 - 16, The test currently uses
inclusive assertions which allow extra ABI entries; update the assertions in
IPausable.test.js to assert exact membership and count by comparing the
sorted/unique names array (variable names from the ABI: abi -> names) to the
exact expected list
['Paused','Unpaused','EnforcedPause','ExpectedPause','paused'] and assert the
length matches that list (or use deep equality on sets/arrays) and remove the
includes/does-not-include checks (replace expect(names).to.include/... and
expect(...).to.not.include/... with a single exact equality/length assertion
against the expected entries).
1 participant
Summary
IPausableto expose thePausableevents, errors, andpaused()getter as a reusable interface artifactPausableinheritIPausableso the implementation stays compiler-checked against the extracted ABI surfaceIPausableartifact contains only the expected Pausable ABI entriesTesting
./node_modules/.bin/hardhat test test/utils/Pausable.test.js test/utils/IPausable.test.js./node_modules/.bin/prettier --check contracts/utils/IPausable.sol contracts/utils/Pausable.sol test/utils/IPausable.test.js./node_modules/.bin/eslint test/utils/IPausable.test.js./node_modules/.bin/solhint --config solhint.config.js --noPoster contracts/utils/IPausable.sol contracts/utils/Pausable.solCloses #5791