-
Notifications
You must be signed in to change notification settings - Fork 30
Fix docs embed reference #196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for confidential-tokens ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR fixes a broken docs embed reference in token.adoc by updating the include path to the correct location, ensuring the referenced content is properly rendered. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughUpdated a docs include and replaced extension-method calls with static Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this 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)
docs/modules/ROOT/pages/token.adoc (1)
100-100: Option: include only the swap function excerpt for consistency with prior example.The ERC7984→ERC7984 snippet uses a lines range; consider excerpting the key function here too.
-include::api:example$SwapERC7984ToERC20.sol[] +include::api:example$SwapERC7984ToERC20.sol[lines=8..24]Adjust the range to match the swap function’s exact lines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/modules/ROOT/pages/token.adoc(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). (4)
- GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
- GitHub Check: slither
- GitHub Check: coverage
- GitHub Check: tests
🔇 Additional comments (1)
docs/modules/ROOT/pages/token.adoc (1)
100-100: Correct include target; aligns with ERC7984-to-ERC20 example. LGTM.The reference now matches the section’s intent. No other issues spotted in the block structure.
| [source,solidity] | ||
| ---- | ||
| include::api:example$SwapConfidentialToERC20.sol[] | ||
| include::api:example$SwapERC7984ToERC20.sol[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify Antora include resolves and remove stale references.
Ensure the example file exists under the expected Antora path (modules/api/examples) and that no docs still reference the old filename.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Verify example file exists:"
fd -a 'SwapERC7984ToERC20.sol'
echo "2) Verify it's under modules/api/examples for Antora family 'example$':"
fd -a 'SwapERC7984ToERC20.sol' | grep -E 'modules/api/examples/SwapERC7984ToERC20\.sol$' || {
echo "Expected path modules/api/examples/SwapERC7984ToERC20.sol not found"; exit 1; }
echo "3) Ensure no stale references remain:"
rg -nS 'SwapConfidentialToERC20\.sol' docs || echo "No stale references."
echo "4) Ensure all includes use the new file:"
rg -nS 'include::api:example\$SwapERC7984ToERC20\.sol' docs || {
echo "No include found for the new file; verify location or module name."; exit 1; }Length of output: 589
Move SwapERC7984ToERC20.sol into docs/modules/api/examples and remove stale references
- Relocate
contracts/mocks/docs/SwapERC7984ToERC20.soltodocs/modules/api/examples/SwapERC7984ToERC20.solsoinclude::api:example$SwapERC7984ToERC20.sol[]resolves correctly. - Search for and delete any lingering mentions of
SwapConfidentialToERC20.sol.
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contracts/mocks/docs/SwapERC7984ToERC20.sol (1)
34-38: Fix custom error usage:requirecannot emit custom errors.
Use explicitrevertwith the custom error.Apply:
- require(to != address(0), SwapConfidentialToERC20InvalidGatewayRequest(requestID)); + if (to == address(0)) { + revert SwapConfidentialToERC20InvalidGatewayRequest(requestID); + }
🧹 Nitpick comments (1)
contracts/mocks/docs/SwapERC7984ToERC20.sol (1)
12-15: Make token references immutable.
Saves gas and documents single-assignment intent.- IERC7984 private _fromToken; - IERC20 private _toToken; + IERC7984 private immutable _fromToken; + IERC20 private immutable _toToken;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
contracts/mocks/docs/ERC7984MintableBurnable.sol(2 hunks)contracts/mocks/docs/SwapERC7984ToERC20.sol(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). (4)
- GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
- GitHub Check: coverage
- GitHub Check: slither
- GitHub Check: tests
🔇 Additional comments (3)
contracts/mocks/docs/ERC7984MintableBurnable.sol (3)
4-4: Import change aligns with static FHE API. LGTM.
Switching to{FHE, externalEuint64}is consistent with replacing extension methods.
20-22: Correct use of FHE.fromExternal for burn.
Same as mint—migration looks good.
16-18: Approve code changes
All mocks now call FHE.fromExternal; no extension-form usages remain.
| euint64 amount = FHE.fromExternal(encryptedInput, inputProof); | ||
| FHE.allowTransient(amount, address(_fromToken)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Right migration: extension → static FHE API.
FHE.fromExternal then FHE.allowTransient(amount, address(_fromToken)) is the correct order before calling _fromToken.confidentialTransferFrom.
Quick scan for any lingering extension-style calls:
🏁 Script executed:
#!/bin/bash
rg -nP '\.\s*(fromExternal|allowTransient)\s*\(' -S -g '!**/node_modules/**'Length of output: 3418
Correct static FHE usage; migrate remaining extension-style calls
The snippet in contracts/mocks/docs/SwapERC7984ToERC20.sol correctly calls
FHE.fromExternal(...);
FHE.allowTransient(...);
before invoking the token transfer.
I ran rg -nP '\.\s*(fromExternal|allowTransient)\s*\(' -S -g '!**/node_modules/**' and uncovered multiple other extension-style FHE calls across the repo. Please update each occurrence to the static FHE API pattern (i.e. FHE.fromExternal(...) immediately followed by FHE.allowTransient(...)) before any _transfer, _mint, _burn, or confidentialTransferFrom calls.
🤖 Prompt for AI Agents
In contracts/mocks/docs/SwapERC7984ToERC20.sol around lines 22-23, the FHE API
is already used correctly (FHE.fromExternal(...) then FHE.allowTransient(...));
however, scan the repo for any extension-style calls like
encryptedInput.fromExternal(...) or amount.allowTransient(...) (use the provided
ripgrep command) and refactor each to the static API pattern: call
FHE.fromExternal(...) and immediately call FHE.allowTransient(...) before any
subsequent _transfer, _mint, _burn, or confidentialTransferFrom invocation;
ensure ordering is preserved and replace the extension-method call sites with
the two static FHE calls so transient allowances are always set prior to token
operations.
Summary by Sourcery
Documentation:
Summary by CodeRabbit