-
Notifications
You must be signed in to change notification settings - Fork 145
fix: update permit hook description #6412
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
base: develop
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughA single text field in the hookDappsRegistry.ts file is refined by removing the word "Infinite" from the PERMIT_HOOK_DAPP_ID entry's descriptionShort field. This is a minimal semantic clarification to the permission description without affecting code logic or structure. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
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: 0
🧹 Nitpick comments (1)
libs/hook-dapp-lib/src/hookDappsRegistry.ts (1)
20-20: Fix duplicated/awkward sentence in validators description (drive‑by copy edit)The sentence repeats and “without requiring ready for withdrawal” is ungrammatical. Suggested cleanup:
- "This hook allows you to withdraw rewards from your Gnosis Chain validators through CoW Swap. It automates the process of interacting with the Gnosis Deposit Contract, enabling you to claim any available rewards directly to your specified withdrawal address. The hook monitors your validator's accrued rewards and triggers the claimWithdrawals function when rewards are ready for withdrawal. This simplifies the management of Gnosis validator earnings without requiring ready for withdrawal. This simplifies the management of Gnosis validator earnings without requiring manual contract interaction, providing a smoother and more efficient experience for users.", + "This hook allows you to withdraw rewards from your Gnosis Chain validators through CoW Swap. It automates interactions with the Gnosis Deposit Contract, enabling you to claim available rewards directly to your specified withdrawal address. The hook monitors your validator's accrued rewards and triggers claimWithdrawals when rewards are ready. This simplifies management of Gnosis validator earnings without manual contract interaction, providing a smoother and more efficient experience.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/hook-dapp-lib/src/hookDappsRegistry.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T05:57:08.021Z
Learnt from: shoom3301
PR: cowprotocol/cowswap#6138
File: libs/hook-dapp-lib/src/hookDappsRegistry.ts:1-1
Timestamp: 2025-08-12T05:57:08.021Z
Learning: The matchHooksToDapps function in libs/hook-dapp-lib/src/utils.ts provides backward compatibility for permit hooks through function selector detection (EIP_2612_PERMIT_SELECTOR and DAI_PERMIT_SELECTOR) rather than dappId matching, making it robust against dappId changes.
Applied to files:
libs/hook-dapp-lib/src/hookDappsRegistry.ts
🔇 Additional comments (2)
libs/hook-dapp-lib/src/hookDappsRegistry.ts (2)
41-43: Confirm if Permit hook should support mainnet beyond Sepolia testnetThe Permit hook (v0.1.0) is currently restricted to Sepolia (11155111) and will not appear on other networks due to filtering in
validateHookDappManifest.tsxanduseInternalHookDapps.ts. Unlike other hooks in the registry which support mainnet and multiple production networks, the Permit hook remains testnet-only. Clarify whether this restriction is intentional pending further testing or oversight—if mainnet support is intended, add chain IDs1and/or others tosupportedNetworksinhookDappsRegistry.tslines 41–43.
33-33: LGTM — change verified and ready to merge; optional: simplify "one token" to "a token" for consistencyThe description update is correct and has no functional impact. No stale "Infinite permit" phrasing exists elsewhere in the codebase. The optional microcopy suggestion—changing "one token" to "a token"—aligns the short description with the entry name and avoids potential ambiguity about quantity.
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.
thanks
Summary
Fixes https://www.notion.so/cownation/Improve-hook-s-description-if-a-permit-is-a-partial-one-not-infinite-25a8da5f04ca804c9f76ecd96d13635d?source=copy_link
To Test
See https://www.notion.so/cownation/Improve-hook-s-description-if-a-permit-is-a-partial-one-not-infinite-25a8da5f04ca804c9f76ecd96d13635d?source=copy_link
Summary by CodeRabbit