Skip to content

Improve 0.8.35 compatibility#6494

Open
Amxx wants to merge 6 commits intoOpenZeppelin:masterfrom
Amxx:update/0.8.35-compatibility
Open

Improve 0.8.35 compatibility#6494
Amxx wants to merge 6 commits intoOpenZeppelin:masterfrom
Amxx:update/0.8.35-compatibility

Conversation

@Amxx
Copy link
Copy Markdown
Collaborator

@Amxx Amxx commented May 5, 2026

  • remove all use of error as a variable name
  • deprecate all function named at
    • add new pos function
    • call the pos functions wherever at was previously called
  • use 0.8.35 by default in hardhat.config.js
    • do not escalate warning to errors for the remaining at functions.

PR Checklist

  • Tests
  • Documentation
  • Changeset entry (run npx changeset add)

Amxx added 4 commits May 4, 2026 18:48
and keep "XXX will do not promote promoted to keyword in the future" warnings to errors.
@Amxx Amxx requested a review from a team as a code owner May 5, 2026 12:18
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 5, 2026

⚠️ No Changeset found

Latest commit: 6d5dcca

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

Walkthrough

This pull request introduces a systematic refactoring of enumerable data structures and parameter naming conventions. The at(...) accessor function across EnumerableSet, EnumerableMap, DoubleEndedQueue, and Checkpoints is replaced with a new pos(...) accessor, with at(...) becoming a deprecated wrapper. Binary search and checkpoint lookup implementations are refactored to use index variables instead of pos. Constructor and function parameters named error are renamed to err across multiple mock contracts and utility libraries to avoid Solidity keyword conflicts. Code generation templates for Checkpoints, EnumerableSet, and EnumerableMap are updated to reflect these changes. The Hardhat Solidity compiler default version is updated from 0.8.31 to 0.8.35.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and clearly summarizes the main objective: updating the codebase for compatibility with Solidity 0.8.35.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description clearly outlines the three main objectives: removing 'error' variable names, deprecating 'at' functions in favor of 'pos', and updating the default Solidity compiler to 0.8.35. These align directly with the changeset modifications across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
contracts/utils/structs/EnumerableMap.sol (1)

292-294: ⚡ Quick win

Use _inner.pos(index) directly inside typed pos(...) wrappers.

These wrappers still call _inner.at(index) indirectly. Switching to _inner.pos(index) keeps the migration consistent and removes deprecated-path usage in new code.

♻️ Proposed change pattern
 function pos(UintToUintMap storage map, uint256 index) internal view returns (uint256 key, uint256 value) {
-    (bytes32 atKey, bytes32 val) = at(map._inner, index);
+    (bytes32 atKey, bytes32 val) = pos(map._inner, index);
     return (uint256(atKey), uint256(val));
 }

Apply the same replacement pattern to the other typed pos(...) wrappers listed above.

Also applies to: 433-435, 574-576, 715-717, 856-858, 1001-1003, 1146-1148, 1287-1289, 1432-1434

🤖 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 `@contracts/utils/structs/EnumerableMap.sol` around lines 292 - 294, The typed
pos wrapper pos(UintToUintMap storage map, uint256 index) currently calls
at(map._inner, index); replace that deprecated indirect call with the new direct
inner method by calling map._inner.pos(index) (or _inner.pos(index)) and cast
the returned (bytes32, bytes32) to (uint256, uint256) as before; apply the same
replacement pattern to the other typed pos(...) wrappers referenced (the
wrappers around _inner.at at the other listed locations) so each uses
_inner.pos(index) directly.
🤖 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 `@scripts/generate/templates/EnumerableMap.js`:
- Around line 291-293: The generated pos function still calls the deprecated at
helper; update the implementation of pos(${name} storage map, uint256 index) to
call pos(map._inner, index) instead of at(map._inner, index), then convert the
returned bytes32 values to the appropriate types using the existing
${fromBytes32} helpers (i.e., use the bytes32 results from pos(...) named atKey
and val and pass them through ${fromBytes32(key.type, 'atKey')} and
${fromBytes32(value.type, 'val')} as before). Ensure the symbol names (pos,
atKey, val, ${fromBytes32}) match the template so generated code no longer
references the deprecated at helper.

---

Nitpick comments:
In `@contracts/utils/structs/EnumerableMap.sol`:
- Around line 292-294: The typed pos wrapper pos(UintToUintMap storage map,
uint256 index) currently calls at(map._inner, index); replace that deprecated
indirect call with the new direct inner method by calling map._inner.pos(index)
(or _inner.pos(index)) and cast the returned (bytes32, bytes32) to (uint256,
uint256) as before; apply the same replacement pattern to the other typed
pos(...) wrappers referenced (the wrappers around _inner.at at the other listed
locations) so each uses _inner.pos(index) directly.
🪄 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: d9c2109a-d8d3-44e4-b601-83575de9def9

📥 Commits

Reviewing files that changed from the base of the PR and between 094c1a1 and e009030.

📒 Files selected for processing (17)
  • contracts/access/extensions/AccessControlEnumerable.sol
  • contracts/governance/utils/Votes.sol
  • contracts/mocks/ConstructorMock.sol
  • contracts/mocks/docs/AccessManagerEnumerable.sol
  • contracts/mocks/token/ERC1155ReceiverMock.sol
  • contracts/mocks/token/ERC1363ReceiverMock.sol
  • contracts/mocks/token/ERC1363SpenderMock.sol
  • contracts/mocks/token/ERC721ReceiverMock.sol
  • contracts/utils/cryptography/ECDSA.sol
  • contracts/utils/structs/Checkpoints.sol
  • contracts/utils/structs/DoubleEndedQueue.sol
  • contracts/utils/structs/EnumerableMap.sol
  • contracts/utils/structs/EnumerableSet.sol
  • hardhat.config.js
  • scripts/generate/templates/Checkpoints.js
  • scripts/generate/templates/EnumerableMap.js
  • scripts/generate/templates/EnumerableSet.js

Comment thread scripts/generate/templates/EnumerableMap.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant