Conversation
🦋 Changeset detectedLatest commit: 5eb995a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
WalkthroughA new changeset was added for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RankifyInstanceMainFacet
participant IRankifyInstance (Event)
User->>RankifyInstanceMainFacet: createGame(params)
RankifyInstanceMainFacet->>IRankifyInstance (Event): emit gameCreated(..., votePhaseDuration, turns)
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
test/RankifyInstance.tsOops! Something went wrong! :( ESLint: 9.18.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@eslint/js' imported from /eslint.config.mjs ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (2)
src/interfaces/IRankifyInstance.sol (1)
33-33: Consider type consistency between input parameter and event parameter.There's a type inconsistency: the
NewGameParamsInput.nTurnsfield isuint96, but the event parameterturnsisuint256. While this works due to implicit upcasting, consider using consistent types for better clarity.- uint256 turns + uint96 turnsOr alternatively, update the input struct to use
uint256for consistency.test/RankifyInstance.ts (1)
1829-1832: Strengthen the assertion by checking event payload, not just countThe test only verifies that some
LastTurnevent for game 1 exists.
If the event is emitted with unexpected parameters (e.g. wrongturnindex, duplicated emission, wrong winner) the test would still pass.-const evts = (await rankifyInstance.queryFilter(rankifyInstance.filters.LastTurn(1))).map(e => e.args); -expect(evts.length).to.be.greaterThan(0); +const evts = await rankifyInstance.queryFilter(rankifyInstance.filters.LastTurn(1)); +expect(evts).to.have.length.greaterThan(0); +// Assert on the first (and normally only) occurrence +const { gameId, turn } = evts[0].args; // adapt to actual signature +expect(gameId).to.equal(1); +expect(turn).to.equal(await rankifyInstance.getTurn(1));Alternatively, capture the tx that should emit the event and use
expect(tx).to.emit(rankifyInstance, 'LastTurn').withArgs(1, expectedTurn), which is both clearer and faster than querying the full log history.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/little-shrimps-love.md(1 hunks)src/facets/RankifyInstanceMainFacet.sol(1 hunks)src/interfaces/IRankifyInstance.sol(1 hunks)test/RankifyInstance.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (2)
.changeset/little-shrimps-love.md (1)
1-6: Changeset format looks good.The changeset properly documents the update with appropriate patch-level versioning. The description accurately reflects the changes made to the event signature and test coverage.
src/facets/RankifyInstanceMainFacet.sol (1)
79-80: gameCreated event signature consistency confirmedThe
gameCreatedemission in src/facets/RankifyInstanceMainFacet.sol now passes the parameters in the exact order defined by the IRankifyInstance interface:• proposingPhaseDuration → params.proposingPhaseDuration
• votePhaseDuration → params.votePhaseDuration
• turns → params.nTurnsNo discrepancies found between the implementation and the interface.
| uint256 votePhaseDuration, | ||
| uint256 turns | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Event signature change is a breaking change - verify compatibility.
The addition of the turns parameter to the gameCreated event is a breaking change that will affect any existing event listeners or indexing systems. Ensure that all dependent systems are updated to handle the new event signature.
🏁 Script executed:
#!/bin/bash
# Description: Search for gameCreated event usage across the codebase to identify potential breaking changes
# Expected: Find all places where gameCreated event is used (listeners, filters, etc.)
echo "Searching for gameCreated event usage..."
rg -A 3 -B 3 "gameCreated" --type sol
rg -A 3 -B 3 "GameCreated" --type ts --type jsLength of output: 203
🏁 Script executed:
#!/bin/bash
# Search for the `gameCreated` event declaration and any references in Solidity files
echo "➡️ Searching for `gameCreated` in Solidity files..."
rg -g '*.sol' -n -A2 -B2 "gameCreated"
# Search for any usage of `gameCreated` in TypeScript/JavaScript for listeners, filters, etc.
echo "➡️ Searching for `gameCreated` in TS/JS files..."
rg -g '*.ts' -g '*.js' -n -A2 -B2 "gameCreated"Length of output: 2938
🚨 Update all references to match the new gameCreated signature
The gameCreated event in src/interfaces/IRankifyInstance.sol now includes two new uint256 parameters—votePhaseDuration and turns—which makes it a breaking change. You’ll need to align every declaration and emit so your mocks, facet, and tests don’t break.
Key locations to fix:
-
src/mocks/RankifyInstanceEventMock.sol
• Line 29: Change theevent gameCreated(...)declaration to includeuint256 votePhaseDuration, uint256 turnsin the correct order.
• Line 72: Update theemit gameCreated(…)call to pass the new parameters (e.g.emit gameCreated(1, address(11), address(14), /*votePhaseDuration*/, /*turns*/);). -
src/facets/RankifyInstanceMainFacet.sol
• Lines 73–75: The currentemit gameCreated(params.gameId, params.gameMaster)only passes two arguments. Addparams.votePhaseDurationandparams.nTurns(cast touint256) in the correct sequence. -
test/RankifyInstance.ts
• Lines 544–547 and 2011–2014: If you use.withArgs(...)after.to.emit(…, 'gameCreated'), include the two new parameters. Ensure your test-setupparamsobject includesnTurnsandvotePhaseDuration.
Type & naming consistency:
- The input struct uses
nTurnsasuint96but the event expectsuint256 turns. To prevent truncation, consider switchingnTurnstouint256. - Unify naming: use
turnsconsistently instead of mixingnTurns,turns, andmaxTurns.
Addressing these changes will ensure your mocks, facet emits, and tests stay in sync with the new event signature.
🤖 Prompt for AI Agents
In src/interfaces/IRankifyInstance.sol around lines 32 to 34, the gameCreated
event signature was updated to include two new uint256 parameters:
votePhaseDuration and turns. To fix this, update the event declaration in
src/mocks/RankifyInstanceEventMock.sol at line 29 to add these parameters in the
correct order, and modify the emit call at line 72 to pass appropriate values
for votePhaseDuration and turns. In src/facets/RankifyInstanceMainFacet.sol
lines 73–75, update the emit gameCreated call to include
params.votePhaseDuration and params.nTurns cast to uint256. In
test/RankifyInstance.ts at lines 544–547 and 2011–2014, update all
.withArgs(...) calls after .to.emit(..., 'gameCreated') to include the new
parameters, and ensure the test params object includes votePhaseDuration and
nTurns. Also, unify the naming by changing nTurns to turns as uint256 to
maintain consistency and prevent truncation.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests