This repository was archived by the owner on Feb 8, 2026. It is now read-only.
🎨 Split verification types#130
Closed
garry-sharp wants to merge 1 commit into
Closed
Conversation
Contributor
WalkthroughThe changes introduce explicit methods for validating plugin policies during creation, update, and deletion operations, replacing previous generic validation calls throughout the codebase. A new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Plugin
Client->>API: Create/Update/Delete PluginPolicy
API->>Plugin: ValidateCreate/Update/DeletePluginPolicy(policy)
Plugin-->>API: Validation result
API-->>Client: Success or 400 Bad Request (on validation error)
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
🧰 Additional context used🧠 Learnings (9)📓 Common learningsapi/server.go (2)plugin/fees/fees.go (9)plugin/fees/policy.go (5)api/plugin.go (3)plugin/payroll/policy.go (5)plugin/dca/dca.go (5)plugin/payroll/payroll.go (8)plugin/common/plugin.go (2)🔇 Additional comments (20)
✨ 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 (
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements the separation of plugin policy validation into different verification types for different contexts (create, update, delete operations). The changes allow plugins to apply context-specific validation rules, such as preventing multiple fee plugins from being installed simultaneously or blocking deletion of fee plugins when fees are still pending collection.
- Introduces a new
Plugininterface inplugin/common/plugin.gowith separate validation methods for create, update, and delete operations - Updates all plugin implementations to implement the new interface methods
- Modifies the API server to use the appropriate validation method based on the operation type
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| plugin/common/plugin.go | Defines new Plugin interface with separate validation methods for different operations |
| plugin/payroll/policy.go | Implements the new validation methods by delegating to existing ValidatePluginPolicy |
| plugin/fees/policy.go | Implements the new validation methods by delegating to existing ValidatePluginPolicy |
| plugin/dca/dca.go | Implements the new validation methods and updates interface reference |
| plugin/payroll/payroll.go | Updates import aliases and interface reference |
| plugin/fees/fees.go | Updates import structure and interface reference |
| api/server.go | Updates Plugin interface reference |
| api/plugin.go | Uses context-specific validation methods in CRUD operations |
Comment on lines
+236
to
+239
| } | ||
| func (p *DCAPlugin) ValidateUpdatePluginPolicy(policyDoc vtypes.PluginPolicy) error { | ||
| return p.ValidatePluginPolicy(policyDoc) | ||
| } |
There was a problem hiding this comment.
[nitpick] Missing spacing between function definitions. Consider adding a blank line after the previous function to improve readability and maintain consistency with the codebase formatting.
Suggested change
| } | |
| func (p *DCAPlugin) ValidateUpdatePluginPolicy(policyDoc vtypes.PluginPolicy) error { | |
| return p.ValidatePluginPolicy(policyDoc) | |
| } | |
| } | |
| func (p *DCAPlugin) ValidateUpdatePluginPolicy(policyDoc vtypes.PluginPolicy) error { | |
| return p.ValidatePluginPolicy(policyDoc) | |
| } |
Comment on lines
+236
to
+239
| } | ||
| func (p *DCAPlugin) ValidateUpdatePluginPolicy(policyDoc vtypes.PluginPolicy) error { | ||
| return p.ValidatePluginPolicy(policyDoc) | ||
| } |
There was a problem hiding this comment.
[nitpick] Missing spacing between function definitions. Consider adding a blank line after the previous function to improve readability and maintain consistency with the codebase formatting.
Suggested change
| } | |
| func (p *DCAPlugin) ValidateUpdatePluginPolicy(policyDoc vtypes.PluginPolicy) error { | |
| return p.ValidatePluginPolicy(policyDoc) | |
| } | |
| } | |
| func (p *DCAPlugin) ValidateUpdatePluginPolicy(policyDoc vtypes.PluginPolicy) error { | |
| return p.ValidatePluginPolicy(policyDoc) | |
| } |
RaghavSood
reviewed
Jul 16, 2025
| "github.com/vultisig/verifier/types" | ||
| ) | ||
|
|
||
| type Plugin interface { |
Contributor
There was a problem hiding this comment.
Hmmm I don't think this works best
The goal of having a verifier/plugin interface is to ensure that we, at a minimum, implement the core functionality that a plugin needs to be functional - separating it out is possible, but I don't think we should do it just yet
If anything, this interface should have the verifier plugin interface embedded in it, not duplicate the functions
|
|
||
| type Plugin interface { | ||
| GetRecipeSpecification() *rtypes.RecipeSchema | ||
|
|
Contributor
There was a problem hiding this comment.
Or we should even consider upstreaming it (but maybe at a later date)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There is a need to be able to verify a policy at different times in different contexts.
For instance:
Use cases are that more than one fee plugin shouldn't be installed at a time, or that the fee plugin shouldn't be deleted if there are still fees pending collection.
Summary by CodeRabbit
New Features
Refactor