Conversation
WalkthroughThis change migrates all references to the Changes
Sequence Diagram(s)sequenceDiagram
participant API
participant Service
participant Storage
participant Plugin
participant VerifierTypes
API->>Service: CreatePolicyWithSync(policy: vtypes.PluginPolicy)
Service->>Storage: InsertPluginPolicyTx(policy: vtypes.PluginPolicy)
Storage->>VerifierTypes: (policy is vtypes.PluginPolicy)
Service->>Plugin: ValidatePluginPolicy(policy: vtypes.PluginPolicy)
Plugin->>VerifierTypes: (policy is vtypes.PluginPolicy)
Service-->>API: (*vtypes.PluginPolicy, error)
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ Finishing Touches
🪧 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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR updates the codebase to pull in the upstream PluginPolicy type from the verifier repository, replacing the internal types.PluginPolicy usage throughout the code. Key changes include:
- Replacing types.PluginPolicy with vtypes.PluginPolicy in function signatures, variable declarations, and error messages.
- Removing the internal PluginPolicy type definition from the internal types package.
- Updating the Go version in the GitHub workflow to 1.24.
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| storage/postgres/policy.go | Updated PluginPolicy types and error messages in Postgres queries. |
| storage/db.go | Updated function signatures to use vtypes.PluginPolicy. |
| service/policy.go | Updated service methods to reflect the new PluginPolicy type. |
| scripts/dev/create_payroll_policy/main.go | Updated PluginPolicy instantiation with vtypes.PluginPolicy. |
| scripts/dev/create_dca_policy/main.go | Updated PluginPolicy instantiation with vtypes.PluginPolicy. |
| plugin/plugin.go | Updated Plugin interface to use vtypes.PluginPolicy. |
| plugin/payroll/transaction.go & policy.go | Updated PluginPolicy references in transactions and validations. |
| plugin/dca/dca.go | Updated PluginPolicy references in validation, transaction proposals. |
| internal/types/policy.go | Removed the internal PluginPolicy type in favor of upstream type. |
| internal/syncer/sync.go | Updated the syncer interface to use vtypes.PluginPolicy. |
| internal/scheduler/scheduler.go | Updated scheduler functions to use vtypes.PluginPolicy. |
| api/plugin.go | Updated API endpoint to use the new PluginPolicy type. |
| .github/workflows/go.yml | Updated the Go version from 1.22 to 1.24. |
Files not reviewed (1)
- go.mod: Language not supported
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
storage/postgres/policy.go (2)
50-66: UpdatedGetAllPluginPoliciesfor external type
All references now use[]vtypes.PluginPolicy. Consider unifying the JSON scanning approach withGetPluginPolicy—either scan into a temporary[]bytethen assign tojson.RawMessage, or scan directly into thePolicyfield for consistency across methods.
92-105: MigratedInsertPluginPolicyTxto external type
The insertion logic correctly marshals and returnsvtypes.PluginPolicy. You might renamepolicyJSONtopolicyBytesfor clearer intent since it's used both for binding and scanning back.plugin/dca/dca.go (2)
384-388: UpdatedValidateProposedTransactionsto use external policy
Signature change is consistent. For better cancellation support, consider propagating the incomingctxinstead of usingcontext.Background()in nested calls (e.g.,getCompletedSwapTransactionsCount).
786-792: SwitchedcompletePolicyto externalPluginPolicyand retained TODO
The signature updates are correct. The TODO for syncing a COMPLETED state with the verifier database remains; if you'd like assistance implementing that sync, let me know.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
flake.lockis excluded by!**/*.lockgo.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
.github/workflows/go.yml(1 hunks)api/plugin.go(5 hunks)go.mod(3 hunks)internal/scheduler/scheduler.go(3 hunks)internal/syncer/sync.go(4 hunks)internal/types/policy.go(0 hunks)plugin/dca/dca.go(6 hunks)plugin/payroll/policy.go(2 hunks)plugin/payroll/transaction.go(4 hunks)plugin/plugin.go(1 hunks)scripts/dev/create_dca_policy/main.go(2 hunks)scripts/dev/create_payroll_policy/main.go(2 hunks)service/policy.go(4 hunks)storage/db.go(2 hunks)storage/postgres/policy.go(7 hunks)
💤 Files with no reviewable changes (1)
- internal/types/policy.go
🧰 Additional context used
🧬 Code Graph Analysis (7)
api/plugin.go (1)
api/server.go (1)
Server(40-56)
plugin/payroll/policy.go (2)
plugin/payroll/payroll.go (1)
PayrollPlugin(16-21)internal/types/keysign.go (1)
PluginKeysignRequest(39-45)
internal/scheduler/scheduler.go (1)
internal/types/time_trigger.go (1)
TimeTrigger(12-21)
plugin/payroll/transaction.go (2)
plugin/payroll/payroll.go (1)
PayrollPlugin(16-21)internal/types/keysign.go (1)
PluginKeysignRequest(39-45)
storage/postgres/policy.go (2)
storage/postgres/db.go (1)
PostgresBackend(21-23)service/policy.go (1)
Policy(16-23)
plugin/dca/dca.go (1)
internal/types/keysign.go (1)
PluginKeysignRequest(39-45)
plugin/plugin.go (1)
internal/types/keysign.go (1)
PluginKeysignRequest(39-45)
🔇 Additional comments (43)
.github/workflows/go.yml (1)
22-22: Go version update looks good.The workflow has been updated to use Go 1.24, which aligns with the Go version update to 1.24.2 in go.mod. This ensures consistency between the CI environment and the updated module requirements.
scripts/dev/create_payroll_policy/main.go (2)
17-17: Import of external types package added correctly.The import of the verifier types package with the alias
vtypesis properly configured.
107-107: Type replacement implemented correctly.The code now correctly uses
vtypes.PluginPolicyfrom the external verifier types package instead of the internal type, which aligns with the PR objective of importing the verifier/types.PluginPolicy component.scripts/dev/create_dca_policy/main.go (2)
17-17: Import of external types package added correctly.The import of the verifier types package with the alias
vtypesis properly configured.
78-78: Type replacement implemented correctly.The code now correctly uses
vtypes.PluginPolicyfrom the external verifier types package instead of the internal type, which aligns with the PR objective of importing the verifier/types.PluginPolicy component.go.mod (4)
3-3: Go version updated appropriately.The Go version has been updated to 1.24.2, which is consistent with the GitHub workflow update.
7-26: Direct dependencies updated correctly.All direct dependencies have been updated to newer versions, which is good practice for security and stability. This includes important dependencies like aws-sdk-go, go-ethereum, validator, and others.
28-63: Indirect dependencies updated correctly.The indirect dependencies have been updated to align with the direct dependency updates. This is particularly important for compatibility with the new Go version and the external verifier types package.
60-60: Verifier dependency added as expected.The dependency on github.com/vultisig/verifier has been added, which is necessary for importing the external PluginPolicy type as mentioned in the PR objectives.
api/plugin.go (5)
22-22: Import added for external PluginPolicy type.The import for the verifier types package is added to support the migration from internal to external PluginPolicy type.
203-203: Type changed from internal to external PluginPolicy.The policy variable is now using
vtypes.PluginPolicyinstead of the internal type, consistent with the PR objective of integrating types from the verifier repository.
268-268: Type changed from internal to external PluginPolicy.The policy variable for UpdatePluginPolicyById method is now using
vtypes.PluginPolicyinstead of the internal type, consistent with the type migration.
469-469: Method signature updated to use external PluginPolicy type.The verifyPolicySignature method now accepts the external
vtypes.PluginPolicytype while maintaining the same functionality.
496-496: Method signature updated to use external PluginPolicy type.The policyToMessageHex helper function now accepts the external
vtypes.PluginPolicytype while maintaining the same functionality for serializing policies.plugin/payroll/policy.go (3)
16-16: Import added for external PluginPolicy type.The import for the verifier types package is added to support the migration from internal to external PluginPolicy type.
19-19: Method signature updated to use external PluginPolicy type.The ValidateProposedTransactions method now accepts the external
vtypes.PluginPolicytype while maintaining the same validation logic.
90-90: Method signature updated to use external PluginPolicy type.The ValidatePluginPolicy method now accepts the external
vtypes.PluginPolicytype while maintaining the same validation logic for payroll policies.internal/scheduler/scheduler.go (3)
18-18: Import added for external PluginPolicy type.The import for the verifier types package is added to support the migration from internal to external PluginPolicy type.
171-171: Method signature updated to use external PluginPolicy type.The CreateTimeTrigger method now accepts the external
vtypes.PluginPolicytype while maintaining the same functionality for creating time-based triggers.
184-184: Method signature updated to use external PluginPolicy type.The GetTriggerFromPolicy helper method now accepts the external
vtypes.PluginPolicytype while maintaining the same logic for extracting trigger information from policies.internal/syncer/sync.go (4)
12-12: Import added for external PluginPolicy type.The import for the verifier types package is added to support the migration from internal to external PluginPolicy type.
32-34: Interface methods updated to use external PluginPolicy type.The PolicySyncer interface methods now use the external
vtypes.PluginPolicytype, ensuring consistency across the codebase.
53-53: Method signature updated to use external PluginPolicy type.The CreatePolicySync implementation now accepts the external
vtypes.PluginPolicytype while maintaining the same functionality for synchronizing policy creation.
95-95: Method signature updated to use external PluginPolicy type.The UpdatePolicySync implementation now accepts the external
vtypes.PluginPolicytype while maintaining the same functionality for synchronizing policy updates.Verify dependency requirements after this PR
Since this PR relies on changes from the verifier repository (PR #14), ensure you run
go mod tidyafter the dependent PR is merged to update all module dependencies correctly.plugin/plugin.go (2)
9-9: Correct import alias for external type.The import alias
vtypesfor the external verifier types package is appropriate and will help distinguish it from the internal types package.
14-17: LGTM: Interface methods properly updated to use external PluginPolicy type.All method signatures in the Plugin interface have been correctly updated to use
vtypes.PluginPolicyinstead of the internal type, while maintaining the same parameter names and return types. This change is consistent with the PR objective of consolidating shared dependencies.storage/db.go (2)
9-10: Import order is consistent with Go standards.The addition of the new import for the external verifier types package is properly aliased as
vtypesand placed appropriately in the import list.
19-23: Interface methods properly updated for PluginPolicy database operations.All database interface methods dealing with the PluginPolicy type have been correctly updated to use the external
vtypes.PluginPolicytype instead of the internal type. This maintains the interface contract while ensuring consistency with the type migration.plugin/payroll/transaction.go (4)
24-24: Consistent import alias used across codebase.The import alias
vtypesfor the external verifier types package matches the pattern used in other files, providing consistency.
33-33: Method signature updated correctly for ProposeTransactions.The parameter type has been properly updated to use the external
vtypes.PluginPolicytype.
210-210: Method signature updated correctly for SigningComplete.The parameter type has been properly updated to use the external
vtypes.PluginPolicytype while maintaining the same method signature and functionality.
253-253: Method signature updated correctly for convertData helper method.The parameter type has been properly updated to use the external
vtypes.PluginPolicytype while preserving the same method signature and return types.service/policy.go (4)
13-13: Import alias consistent with other files.The import alias
vtypesfor the external verifier types package follows the same pattern used across the codebase.
17-22: Interface methods correctly updated to use external PluginPolicy type.All method signatures in the Policy interface have been properly updated to use
vtypes.PluginPolicyinstead of the internal type, maintaining consistent method contracts.
44-44: Implementation methods correctly updated to match interface changes.All PolicyService implementation methods have been properly updated to use the external
vtypes.PluginPolicytype, ensuring consistency with the interface definition.Also applies to: 79-79, 143-143, 151-151
154-154: Empty struct initialization updated to use the external type.The error case correctly returns an empty
vtypes.PluginPolicy{}struct instead of the internal type, maintaining consistency with the return type change.storage/postgres/policy.go (3)
11-11: Importing external plugin policy type
The aliasvtypesforgithub.com/vultisig/verifier/typesis clear and appropriate. It aligns imports with the upstream dependency.
14-19: SwitchedGetPluginPolicyto externalPluginPolicy
The method signature and default error return correctly usevtypes.PluginPolicy. Error wrapping and JSON assignment remain consistent with the existing logic.
140-158: Review field updates inUpdatePluginPolicyTx
Only a subset of fields (public_key,plugin_type,signature,active,policy) are updated and the// TODO: update other fieldsremains. Please confirm whether fields likechain_code_hex,derive_path, or version fields should also be mutable. Remove or address the TODO once confirmed.plugin/dca/dca.go (4)
31-31: Imported externalPluginPolicytype
The addition of thevtypesimport and removal of internal policy references aligns this plugin with the upstreamverifier/typesdefinitions.
98-103: SwitchedSigningCompleteto externalPluginPolicy
The method signature update is correct. Ensure all invocations ofSigningCompletehave been updated to pass the new type.
145-150: UpdatedValidatePluginPolicysignature to usevtypes.PluginPolicy
Signature and validation logic remain intact. The checks for plugin type, version, and JSON unmarshalling continue to operate on the external policy type.
301-305: MigratedProposeTransactionsto externalPluginPolicy
Signature and initial logging now adoptvtypes.PluginPolicy. The downstream JSON unmarshal and business logic are unaffected by the type swap.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
storage/db.go (1)
9-9: Consider reviewing internal types packageThe static analysis hints that there might be unused imports in the internal types package, likely due to the removal of the internal
PluginPolicytype.After this PR is merged, consider reviewing the internal types package to clean up any unused imports.
🧰 Tools
🪛 golangci-lint (1.64.8)
9-9: could not import github.com/vultisig/vultiserver-plugin/internal/types (-: # github.com/vultisig/vultiserver-plugin/internal/types
internal/types/policy.go:3:8: "encoding/json" imported and not used)(typecheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (14)
api/plugin.go(5 hunks)go.mod(2 hunks)internal/scheduler/scheduler.go(3 hunks)internal/syncer/sync.go(4 hunks)internal/types/policy.go(0 hunks)plugin/dca/dca.go(6 hunks)plugin/payroll/policy.go(2 hunks)plugin/payroll/transaction.go(4 hunks)plugin/plugin.go(1 hunks)scripts/dev/create_dca_policy/main.go(2 hunks)scripts/dev/create_payroll_policy/main.go(2 hunks)service/policy.go(4 hunks)storage/db.go(2 hunks)storage/postgres/policy.go(7 hunks)
💤 Files with no reviewable changes (1)
- internal/types/policy.go
✅ Files skipped from review due to trivial changes (1)
- scripts/dev/create_payroll_policy/main.go
🚧 Files skipped from review as they are similar to previous changes (10)
- scripts/dev/create_dca_policy/main.go
- internal/scheduler/scheduler.go
- api/plugin.go
- plugin/payroll/policy.go
- service/policy.go
- plugin/dca/dca.go
- storage/postgres/policy.go
- plugin/payroll/transaction.go
- plugin/plugin.go
- go.mod
🧰 Additional context used
🪛 GitHub Check: build
internal/syncer/sync.go
[failure] 33-33:
undefined: vtypes.PluginPolicy
[failure] 32-32:
undefined: vtypes.PluginPolicy
[failure] 53-53:
undefined: vtypes.PluginPolicy
[failure] 95-95:
undefined: vtypes.PluginPolicy
🪛 golangci-lint (1.64.8)
storage/db.go
9-9: could not import github.com/vultisig/vultiserver-plugin/internal/types (-: # github.com/vultisig/vultiserver-plugin/internal/types
internal/types/policy.go:3:8: "encoding/json" imported and not used)
(typecheck)
19-19: undefined: vtypes.PluginPolicy
(typecheck)
20-20: undefined: vtypes.PluginPolicy
(typecheck)
22-22: undefined: vtypes.PluginPolicy
(typecheck)
🔇 Additional comments (5)
internal/syncer/sync.go (3)
12-12: Import of externalvtypespackage aligns with PR objectivesThis addition correctly imports the
PluginPolicytype from the verifier repository, which is the first step in consolidating the shared dependencies between repositories as described in the PR objectives.
32-34: Interface method signatures updated correctlyThe
PolicySyncerinterface has been properly updated to use the externalvtypes.PluginPolicytype instead of the internal implementation.Note that the static analysis indicates undefined
vtypes.PluginPolicyerrors, which is expected since this PR depends on a merge in the verifier repository (PR #14). These errors will be resolved by runninggo mod tidyafter that PR is merged.🧰 Tools
🪛 GitHub Check: build
[failure] 33-33:
undefined: vtypes.PluginPolicy
[failure] 32-32:
undefined: vtypes.PluginPolicy
53-53: Implementation method signatures updated consistentlyThe implementation methods in the
Syncerstruct now correctly use the externalvtypes.PluginPolicytype, maintaining consistency with the interface definition.Also applies to: 95-95
🧰 Tools
🪛 GitHub Check: build
[failure] 53-53:
undefined: vtypes.PluginPolicystorage/db.go (2)
9-10: Import statements correctly updatedBoth the internal types package and the external verifier types package are properly imported. The internal package is still needed for other type definitions while the external package provides the
PluginPolicytype.🧰 Tools
🪛 golangci-lint (1.64.8)
9-9: could not import github.com/vultisig/vultiserver-plugin/internal/types (-: # github.com/vultisig/vultiserver-plugin/internal/types
internal/types/policy.go:3:8: "encoding/json" imported and not used)(typecheck)
19-23:Details
✅ Verification successful
Database interface methods correctly updated
All methods in the
DatabaseStorageinterface that interact with plugin policies now use the externalvtypes.PluginPolicytype instead of the internal implementation.The static analysis indicates undefined
vtypes.PluginPolicyerrors, which is expected since this PR depends on a merge in the verifier repository (PR #14). Ensure you rungo mod tidyafter that PR is merged to resolve these dependency issues.
🏁 Script executed:
#!/bin/bash # This will check the current module requirements go list -m all | grep vultisig/verifier # After the verifier PR is merged, you'll need to run: # go mod tidyLength of output: 144
Dependencies updated — run
go mod tidyConfirmed that your project is now pulling in the merged verifier dependency:
github.com/vultisig/verifier v0.0.0-20250504225813-a24e50ad336bAfter PR #14 in the verifier repo is merged, simply run:
go mod tidyto resolve the previously undefined
vtypes.PluginPolicyerrors.🧰 Tools
🪛 golangci-lint (1.64.8)
19-19: undefined: vtypes.PluginPolicy
(typecheck)
20-20: undefined: vtypes.PluginPolicy
(typecheck)
22-22: undefined: vtypes.PluginPolicy
(typecheck)
This is a stacked PR over #17.
Resolves #14
This starts the process of pulling in upstream dependencies that we want to come from the verifier repo into plugins.
To begin with, we're simply pulling in the
verifier/types.PluginPolicy.This requires vultisig/verifier#14 to be merged, and will need a
go mod tidyafter that is doneSummary by CodeRabbit
Refactor
Chores