Skip to content
This repository was archived by the owner on Feb 8, 2026. It is now read-only.

🐛 Small bug fixes for fees#131

Merged
garry-sharp merged 4 commits into
mainfrom
small-bug-fixes-fees
Jul 23, 2025
Merged

🐛 Small bug fixes for fees#131
garry-sharp merged 4 commits into
mainfrom
small-bug-fixes-fees

Conversation

@garry-sharp

@garry-sharp garry-sharp commented Jul 17, 2025

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features
    • Added configuration options for specifying a USDC token address and a verifier token key.
    • Introduced a detailed verifier configuration section with URL, token, and party prefix.
  • Bug Fixes
    • Updated block storage service URLs in configuration examples to use the correct host.
  • Refactor
    • Simplified transaction handling by using a fixed USDC token address from configuration for Ethereum transactions.
    • Added method to retrieve scheduling information from policy configurations.
    • Refactored configuration loading and verifier integration in fee plugin initialization.
  • Documentation
    • Updated example configuration files to reflect new service endpoints and verifier settings.

Copilot AI review requested due to automatic review settings July 17, 2025 14:22
@coderabbitai

coderabbitai Bot commented Jul 17, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

This change introduces new configuration fields for USDC token address and verifier token in both code and configuration files. It updates the fee plugin logic to use these fields directly, removing dynamic parsing of token addresses. Additionally, block storage service URLs in example config files are updated, and a new scheduler utility method is added.

Changes

Files/Group Change Summary
etc/vultisig/fee.yml, plugin/fees/config.go Added usdc_address and verifier_token fields to config file and struct.
plugin/fees/transaction.go Refactored transaction logic to use hardcoded USDC address from config; removed dynamic parsing.
fee.server.example.json, payroll.worker.example.json, fee.worker.example.json Updated block storage host URL from localhost to minio-plugin:9000; restructured verifier config.
internal/scheduler/scheduler.go Added GetScheduleFromPolicy method to retrieve schedule from policy.
api/server_config.go Removed VerifierUrl field from ServerConfig struct.
cmd/fees/config/config.go Added verifier struct and included it in CoreConfig.
cmd/fees/server/config.go Deleted configuration loading code and CoreConfig struct.
cmd/fees/server/main.go Updated import and usage of configuration; passed new signer argument to fee plugin constructor.
cmd/fees/worker/main.go Created and initialized new signer; updated fee plugin constructor call and config usage.
plugin/fees/fees.go Added signer field to FeePlugin struct and constructor parameter.
go.mod Updated github.com/vultisig/verifier module version.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant FeePlugin
    participant Config
    participant TransactionIndexer

    User->>FeePlugin: ProposeTransactions(policy)
    FeePlugin->>Config: Get USDC address, verifier token
    FeePlugin->>TransactionIndexer: Use USDC address for transaction lookup/creation
    TransactionIndexer-->>FeePlugin: Transaction result
    FeePlugin-->>User: Response
Loading

Estimated code review effort

3 (30–60 minutes)

Possibly related PRs

Suggested reviewers

  • johnnyluo

Poem

A hop and a skip, new configs appear,
USDC and tokens, the purpose is clear.
Block storage now points where Minio stays,
Fee logic simplified, no parsing maze.
Schedules unveiled with a fresh new call—
A rabbit approves, with carrots for all! 🥕

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces small bug fixes and enhancements for the fees plugin, including inlining USDC token handling, adding new configuration fields, and improving policy parsing.

  • Inline a hard-coded USDC token instead of dynamic resolution
  • Add UsdcAddress and VerifierToken to configuration
  • Introduce GetScheduleFromPolicy helper and bump module versions
  • Update example storage hosts in JSON and YAML configs

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
plugin/fees/transaction.go Inlines USDC token setup, removes dynamic token logic, and updates token address usage
plugin/fees/config.go Adds UsdcAddress and VerifierToken config fields
internal/scheduler/scheduler.go Adds GetScheduleFromPolicy to extract schedule from policy
payroll.worker.example.json Updates block_storage host to minio-plugin:9000
fee.server.example.json Updates block_storage host to minio-plugin:9000
etc/vultisig/fee.yml Adds usdc_address and verifier_token entries
go.mod Bumps github.com/vultisig/recipes to new version
Comments suppressed due to low confidence (4)

plugin/fees/transaction.go:41

  • [nitpick] The comment mentions a token list even though only a single USDC token is defined; consider updating or removing the 'List' suffix for clarity.
	// ERC20 USDC Token List

internal/scheduler/scheduler.go:188

  • [nitpick] Error message wraps the base64 decoding error with the function call name in uppercase; consider using a lowercase, descriptive prefix like 'failed to decode recipe:' to follow Go error conventions.
	recipe, err := base64.StdEncoding.DecodeString(policy.Recipe)

plugin/fees/transaction.go:70

  • Missing default case for unsupported rule.Resource values; adding an explicit error for unrecognized resources would clarify failure scenarios instead of relying on an empty recipient check.
		switch rule.Resource {

plugin/fees/config.go:38

  • The new VerifierToken configuration field is not used in the transaction plugin; ensure it's integrated into the verifierApi calls or remove it if unnecessary.
	VerifierToken               string   `mapstructure:"verifier_token"`                // The token to use for the verifier API.

Comment thread plugin/fees/transaction.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 (5)
etc/vultisig/fee.yml (1)

9-9: Add missing newline at end of file.

The YAML file should end with a newline character to comply with formatting standards.

-verifier_token: localhost-fee-key
+verifier_token: localhost-fee-key
+
plugin/fees/config.go (2)

43-50: Consider adding default values for new configuration fields.

The withDefaults function doesn't set default values for the new UsdcAddress and VerifierToken fields. Consider whether these should have sensible defaults.

 func withDefaults(c *FeeConfig) {
 	c.Type = PLUGIN_TYPE
 	c.Version = "1.0.0"
 	c.RpcURL = "https://ethereum.publicnode.com/"
 	c.CollectorWhitelistAddresses = []string{}
 	c.CollectorAddress = ""
 	c.MaxFeeAmount = 500e6 // 500 USDC
+	c.UsdcAddress = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48" // Mainnet USDC
+	c.VerifierToken = ""
 }

118-141: Consider adding validation for new configuration fields.

The validation logic doesn't check the new UsdcAddress and VerifierToken fields. Consider whether these should be validated (e.g., UsdcAddress format, VerifierToken presence).

 	// Collector address must be in the whitelist
 	if !slices.Contains(c.CollectorWhitelistAddresses, c.CollectorAddress) {
 		return c, fmt.Errorf("collector_address must be in the whitelist: %s, whitelist: %v", c.CollectorAddress, c.CollectorWhitelistAddresses)
 	}
+
+	// Validate USDC address format (basic check)
+	if c.UsdcAddress != "" && !strings.HasPrefix(c.UsdcAddress, "0x") {
+		return c, fmt.Errorf("invalid USDC address format: %s", c.UsdcAddress)
+	}
 
 	return c, nil
 }
internal/scheduler/scheduler.go (1)

187-198: Consider refactoring to reduce code duplication.

The GetScheduleFromPolicy method duplicates the base64 decoding and protobuf unmarshaling logic from GetTriggerFromPolicy. This creates maintenance overhead and potential for inconsistencies.

Consider extracting the common logic into a helper method:

+func (s *SchedulerService) parsePolicy(policy vtypes.PluginPolicy) (*rtypes.Policy, error) {
+	recipe, err := base64.StdEncoding.DecodeString(policy.Recipe)
+	if err != nil {
+		return nil, fmt.Errorf("base64.StdEncoding.DecodeString: %w", err)
+	}
+
+	var p rtypes.Policy
+	if err := proto.Unmarshal(recipe, &p); err != nil {
+		return nil, fmt.Errorf("failed to parse policy schedule: %w", err)
+	}
+	return &p, nil
+}
+
func (s *SchedulerService) GetScheduleFromPolicy(policy vtypes.PluginPolicy) (*rtypes.Schedule, error) {
-	recipe, err := base64.StdEncoding.DecodeString(policy.Recipe)
-	if err != nil {
-		return nil, fmt.Errorf("base64.StdEncoding.DecodeString: %w", err)
-	}
-
-	var p rtypes.Policy
-	if err := proto.Unmarshal(recipe, &p); err != nil {
-		return nil, fmt.Errorf("failed to parse policy schedule: %w", err)
-	}
+	p, err := s.parsePolicy(policy)
+	if err != nil {
+		return nil, err
+	}
-	return p.Schedule, nil
+	return p.Schedule, nil
}

Then update GetTriggerFromPolicy to use the same helper method.

plugin/fees/transaction.go (1)

70-71: Good change to use resource-based matching.

Switching from rule.Id to rule.Resource is more semantic and aligns with the fee plugin's specific handling of "ethereum.usdc.transfer" resources.

Consider updating the error message on line 82 to be consistent:

-		return nil, fmt.Errorf("unsupported rule: %v", rule.Id)
+		return nil, fmt.Errorf("unsupported resource: %v", rule.Resource)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 88d77cf and 226b17e.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (7)
  • etc/vultisig/fee.yml (1 hunks)
  • fee.server.example.json (1 hunks)
  • go.mod (1 hunks)
  • internal/scheduler/scheduler.go (1 hunks)
  • payroll.worker.example.json (1 hunks)
  • plugin/fees/config.go (1 hunks)
  • plugin/fees/transaction.go (5 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: garry-sharp
PR: vultisig/plugin#117
File: plugin/fees/constraints.go:3-3
Timestamp: 2025-07-04T10:50:42.832Z
Learning: For the vultisig/plugin project, use "fee" (singular) throughout the codebase wherever possible, not "fees" (plural). This applies to all naming conventions including plugin types, variable names, function names, comments, and documentation.
Learnt from: garry-sharp
PR: vultisig/plugin#117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.
etc/vultisig/fee.yml (3)
Learnt from: webpiratt
PR: vultisig/plugin#126
File: plugin/fees/policy.go:26-26
Timestamp: 2025-07-10T20:41:44.025Z
Learning: The constant `vtypes.PluginVultisigFees_feee` in the vultisig/plugin project is correctly spelled with "feee" (including the extra 'e'). This is the actual constant name defined in the external dependency github.com/vultisig/verifier/types and should not be changed to "fee" singular, as this is how it's defined in the external package.
Learnt from: garry-sharp
PR: vultisig/plugin#117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.
Learnt from: garry-sharp
PR: vultisig/plugin#117
File: plugin/fees/constraints.go:3-3
Timestamp: 2025-07-04T10:50:42.832Z
Learning: For the vultisig/plugin project, use "fee" (singular) throughout the codebase wherever possible, not "fees" (plural). This applies to all naming conventions including plugin types, variable names, function names, comments, and documentation.
go.mod (2)
Learnt from: RaghavSood
PR: vultisig/plugin#36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path `github.com/vultisig/verifier/plugin` refers to an external dependency that provides the plugin interface, and should not be changed to `github.com/vultisig/plugin/plugin` as these are distinct packages with different purposes.
Learnt from: johnnyluo
PR: vultisig/plugin#108
File: Dockerfile.Payroll.server:14-19
Timestamp: 2025-07-02T04:55:36.331Z
Learning: In the vultisig/plugin repository, the team maintains both the main repository and the go-wrappers dependency repository, so they are comfortable downloading from the master branch rather than pinning to specific commits.
plugin/fees/config.go (5)
Learnt from: garry-sharp
PR: vultisig/plugin#117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.
Learnt from: webpiratt
PR: vultisig/plugin#126
File: plugin/fees/policy.go:26-26
Timestamp: 2025-07-10T20:41:44.025Z
Learning: The constant `vtypes.PluginVultisigFees_feee` in the vultisig/plugin project is correctly spelled with "feee" (including the extra 'e'). This is the actual constant name defined in the external dependency github.com/vultisig/verifier/types and should not be changed to "fee" singular, as this is how it's defined in the external package.
Learnt from: RaghavSood
PR: vultisig/plugin#36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path `github.com/vultisig/verifier/plugin` refers to an external dependency that provides the plugin interface, and should not be changed to `github.com/vultisig/plugin/plugin` as these are distinct packages with different purposes.
Learnt from: johnnyluo
PR: vultisig/plugin#108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.
Learnt from: johnnyluo
PR: vultisig/plugin#108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). It contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.
plugin/fees/transaction.go (11)
Learnt from: garry-sharp
PR: vultisig/plugin#117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.
Learnt from: webpiratt
PR: vultisig/plugin#125
File: plugin/payroll/policy.go:160-160
Timestamp: 2025-07-09T22:23:17.348Z
Learning: In plugin/payroll/policy.go, the "token" case in the checkRule method intentionally does not call validateToken() as this is a temporary implementation that will be replaced with generic schema validation. The missing validation call is expected behavior during this transition period.
Learnt from: webpiratt
PR: vultisig/plugin#126
File: plugin/fees/policy.go:26-26
Timestamp: 2025-07-10T20:41:44.025Z
Learning: The constant `vtypes.PluginVultisigFees_feee` in the vultisig/plugin project is correctly spelled with "feee" (including the extra 'e'). This is the actual constant name defined in the external dependency github.com/vultisig/verifier/types and should not be changed to "fee" singular, as this is how it's defined in the external package.
Learnt from: webpiratt
PR: vultisig/plugin#96
File: plugin/payroll/transaction.go:43-44
Timestamp: 2025-06-18T18:22:06.358Z
Learning: In the vultisig/plugin codebase, the hardcoded ethereumEvmChainID = big.NewInt(1) in plugin/payroll/transaction.go is intentional for the current implementation phase. The team is implementing ETH first, with plans to add other EVM chains later. The functions/methods are already designed to work with all EVM chains.
Learnt from: webpiratt
PR: vultisig/plugin#125
File: plugin/payroll/transaction.go:186-197
Timestamp: 2025-07-09T22:22:36.651Z
Learning: In plugin/payroll/transaction.go, the getTokenID function is designed to return evm.ZeroAddress.Hex() when no "token" parameter constraint is found, rather than returning an error. This is expected behavior according to the team's design intent, even though the token parameter is marked as required in the recipe specification.
Learnt from: webpiratt
PR: vultisig/plugin#96
File: plugin/payroll/transaction.go:0-0
Timestamp: 2025-06-18T18:23:20.077Z
Learning: In the payroll plugin (plugin/payroll/transaction.go), signRequest.Transaction is stored with the "0x" prefix, making it compatible with gcommon.FromHex which requires 0x-prefixed hex strings.
Learnt from: garry-sharp
PR: vultisig/plugin#117
File: plugin/fees/constraints.go:3-3
Timestamp: 2025-07-04T10:50:42.832Z
Learning: For the vultisig/plugin project, use "fee" (singular) throughout the codebase wherever possible, not "fees" (plural). This applies to all naming conventions including plugin types, variable names, function names, comments, and documentation.
Learnt from: webpiratt
PR: vultisig/plugin#105
File: plugin/dca/dca.go:734-736
Timestamp: 2025-07-01T17:35:35.277Z
Learning: The DCA plugin in the vultisig/plugin codebase is currently not fully implemented. Methods like getCompletedSwapTransactionsCount() return placeholder values (e.g., 0) with TODO comments, which is expected behavior during the development phase. The plugin implementation is intentionally incomplete while other parts of the system are being refactored.
Learnt from: webpiratt
PR: vultisig/plugin#96
File: plugin/payroll/transaction.go:510-514
Timestamp: 2025-06-18T18:20:59.510Z
Learning: The erc20ABI constant is defined in plugin/payroll/constants.go within the payroll package, making it accessible to other files in the same package like transaction.go.
Learnt from: RaghavSood
PR: vultisig/plugin#36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path `github.com/vultisig/verifier/plugin` refers to an external dependency that provides the plugin interface, and should not be changed to `github.com/vultisig/plugin/plugin` as these are distinct packages with different purposes.
Learnt from: johnnyluo
PR: vultisig/plugin#108
File: Dockerfile.Payroll.server:14-19
Timestamp: 2025-07-02T04:55:36.331Z
Learning: In the vultisig/plugin repository, the team maintains both the main repository and the go-wrappers dependency repository, so they are comfortable downloading from the master branch rather than pinning to specific commits.
🧬 Code Graph Analysis (1)
internal/scheduler/scheduler.go (1)
plugin/dca/policy.go (1)
  • Schedule (20-25)
🪛 YAMLlint (1.37.1)
etc/vultisig/fee.yml

[error] 9-9: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (6)
payroll.worker.example.json (1)

16-16: LGTM: Block storage host updated to MinIO plugin.

The configuration change from localhost to the containerized MinIO service is consistent with the infrastructure updates.

fee.server.example.json (1)

18-18: LGTM: Block storage host updated to MinIO plugin.

The configuration change is consistent with the infrastructure updates across services.

go.mod (1)

22-22: LGTM: Recipes dependency updated.

The dependency update supports the new USDC and verifier token configurations introduced in this PR.

plugin/fees/config.go (1)

37-38: LGTM: New configuration fields added.

The UsdcAddress and VerifierToken fields are properly defined with appropriate mapstructure tags and documentation.

plugin/fees/transaction.go (2)

41-48: LGTM! Hardcoded USDC token definition is appropriate.

The hardcoded USDC token definition using fp.config.UsdcAddress is a good approach that:

  • Simplifies the code by removing dynamic token lookup dependencies
  • Uses configuration-driven token address for flexibility
  • Correctly defines USDC properties for Ethereum mainnet (6 decimals, proper name/symbol)

This aligns with the fee plugin's focus on USDC-only fee collection.


104-104: Consistent usage of hardcoded USDC address.

The consistent replacement of dynamic token lookups with usdc.Address throughout the method is well-implemented:

  • Transaction indexer calls use the configured address
  • Logging includes the correct token address
  • Transaction creation uses the same address source
  • Maintains functional consistency with the previous dynamic approach

Also applies to: 114-114, 122-122, 134-134

Comment thread etc/vultisig/fee.yml
@garry-sharp
garry-sharp force-pushed the small-bug-fixes-fees branch 2 times, most recently from 7b4cdfd to 8ef2dc8 Compare July 17, 2025 14:34
@garry-sharp
garry-sharp requested a review from RaghavSood July 18, 2025 09:26
@garry-sharp
garry-sharp force-pushed the small-bug-fixes-fees branch from 95dd122 to 7fe2dda Compare July 21, 2025 13:37

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
cmd/fees/config/config.go (1)

22-22: Configuration field integration looks correct.

The Verifier field is properly integrated into the CoreConfig struct, though it's missing the mapstructure tag for consistency with other fields.

Apply this diff to add the missing mapstructure tag:

-	Verifier verifier
+	Verifier verifier `mapstructure:"verifier"`
plugin/fees/fees.go (1)

12-12: Integration looks consistent, but clarify the nature of changes.

The keysign.Signer integration is well-structured and follows Go conventions. However, the PR title mentions "bug fixes" while these changes appear to be adding new signing functionality. Consider updating the PR description to better reflect the nature of these changes.

Also applies to: 38-38, 51-51, 100-100

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7fe2dda and 34deed8.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (10)
  • api/server_config.go (0 hunks)
  • cmd/fees/config/config.go (2 hunks)
  • cmd/fees/server/config.go (0 hunks)
  • cmd/fees/server/main.go (4 hunks)
  • cmd/fees/worker/main.go (4 hunks)
  • fee.server.example.json (2 hunks)
  • fee.worker.example.json (1 hunks)
  • go.mod (1 hunks)
  • plugin/fees/fees.go (5 hunks)
  • plugin/fees/transaction.go (7 hunks)
🧠 Learnings (7)
📓 Common learnings
Learnt from: garry-sharp
PR: vultisig/plugin#117
File: plugin/fees/constraints.go:3-3
Timestamp: 2025-07-04T10:50:42.832Z
Learning: For the vultisig/plugin project, use "fee" (singular) throughout the codebase wherever possible, not "fees" (plural). This applies to all naming conventions including plugin types, variable names, function names, comments, and documentation.
Learnt from: garry-sharp
PR: vultisig/plugin#117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.
fee.server.example.json (4)

Learnt from: RaghavSood
PR: #36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path github.com/vultisig/verifier/plugin refers to an external dependency that provides the plugin interface, and should not be changed to github.com/vultisig/plugin/plugin as these are distinct packages with different purposes.

Learnt from: garry-sharp
PR: #113
File: docker-compose.yaml:80-81
Timestamp: 2025-07-03T15:55:04.605Z
Learning: For docker-compose.yaml files used for local development builds, hardcoded database credentials like "myuser:mypassword" in DATABASE_DSN environment variables are acceptable since they're only used locally and not exposed to external networks.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

Learnt from: webpiratt
PR: #127
File: internal/keysign/api_emitter.go:12-21
Timestamp: 2025-07-19T11:09:26.168Z
Learning: In the vultisig/plugin codebase, URL formatting and validation for endpoints like the verifier emitter is handled at the configuration level, so additional URL validation in individual functions is not needed.

cmd/fees/config/config.go (5)

Learnt from: RaghavSood
PR: #36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path github.com/vultisig/verifier/plugin refers to an external dependency that provides the plugin interface, and should not be changed to github.com/vultisig/plugin/plugin as these are distinct packages with different purposes.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.

Learnt from: webpiratt
PR: #126
File: plugin/fees/policy.go:26-26
Timestamp: 2025-07-10T20:41:44.025Z
Learning: The constant vtypes.PluginVultisigFees_feee in the vultisig/plugin project is correctly spelled with "feee" (including the extra 'e'). This is the actual constant name defined in the external dependency github.com/vultisig/verifier/types and should not be changed to "fee" singular, as this is how it's defined in the external package.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). It contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

cmd/fees/server/main.go (7)

Learnt from: RaghavSood
PR: #36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path github.com/vultisig/verifier/plugin refers to an external dependency that provides the plugin interface, and should not be changed to github.com/vultisig/plugin/plugin as these are distinct packages with different purposes.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.

Learnt from: webpiratt
PR: #126
File: plugin/fees/policy.go:26-26
Timestamp: 2025-07-10T20:41:44.025Z
Learning: The constant vtypes.PluginVultisigFees_feee in the vultisig/plugin project is correctly spelled with "feee" (including the extra 'e'). This is the actual constant name defined in the external dependency github.com/vultisig/verifier/types and should not be changed to "fee" singular, as this is how it's defined in the external package.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/constraints.go:3-3
Timestamp: 2025-07-04T10:50:42.832Z
Learning: For the vultisig/plugin project, use "fee" (singular) throughout the codebase wherever possible, not "fees" (plural). This applies to all naming conventions including plugin types, variable names, function names, comments, and documentation.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). It contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

Learnt from: RaghavSood
PR: #75
File: storage/postgres/db_plugin.go:25-40
Timestamp: 2025-05-30T02:44:31.711Z
Learning: The embed.FS directive //go:embed migrations/plugin/*.sql in storage/postgres/db_plugin.go correctly embeds plugin migration files that exist in the storage/postgres/migrations/plugin/ directory. The path "migrations/plugin" passed to goose.Up() references the embedded filesystem structure, not the physical file system path.

fee.worker.example.json (2)

Learnt from: RaghavSood
PR: #36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path github.com/vultisig/verifier/plugin refers to an external dependency that provides the plugin interface, and should not be changed to github.com/vultisig/plugin/plugin as these are distinct packages with different purposes.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

plugin/fees/fees.go (9)

Learnt from: webpiratt
PR: #126
File: plugin/fees/policy.go:26-26
Timestamp: 2025-07-10T20:41:44.025Z
Learning: The constant vtypes.PluginVultisigFees_feee in the vultisig/plugin project is correctly spelled with "feee" (including the extra 'e'). This is the actual constant name defined in the external dependency github.com/vultisig/verifier/types and should not be changed to "fee" singular, as this is how it's defined in the external package.

Learnt from: RaghavSood
PR: #36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path github.com/vultisig/verifier/plugin refers to an external dependency that provides the plugin interface, and should not be changed to github.com/vultisig/plugin/plugin as these are distinct packages with different purposes.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/constraints.go:3-3
Timestamp: 2025-07-04T10:50:42.832Z
Learning: For the vultisig/plugin project, use "fee" (singular) throughout the codebase wherever possible, not "fees" (plural). This applies to all naming conventions including plugin types, variable names, function names, comments, and documentation.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.

Learnt from: webpiratt
PR: #96
File: plugin/payroll/transaction.go:0-0
Timestamp: 2025-06-18T18:23:20.077Z
Learning: In the payroll plugin (plugin/payroll/transaction.go), signRequest.Transaction is stored with the "0x" prefix, making it compatible with gcommon.FromHex which requires 0x-prefixed hex strings.

Learnt from: johnnyluo
PR: #108
File: Dockerfile.Payroll.server:14-19
Timestamp: 2025-07-02T04:55:36.331Z
Learning: In the vultisig/plugin repository, the team maintains both the main repository and the go-wrappers dependency repository, so they are comfortable downloading from the master branch rather than pinning to specific commits.

Learnt from: webpiratt
PR: #96
File: plugin/payroll/transaction.go:43-44
Timestamp: 2025-06-18T18:22:06.358Z
Learning: In the vultisig/plugin codebase, the hardcoded ethereumEvmChainID = big.NewInt(1) in plugin/payroll/transaction.go is intentional for the current implementation phase. The team is implementing ETH first, with plans to add other EVM chains later. The functions/methods are already designed to work with all EVM chains.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). It contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

cmd/fees/worker/main.go (7)

Learnt from: webpiratt
PR: #126
File: plugin/fees/policy.go:26-26
Timestamp: 2025-07-10T20:41:44.025Z
Learning: The constant vtypes.PluginVultisigFees_feee in the vultisig/plugin project is correctly spelled with "feee" (including the extra 'e'). This is the actual constant name defined in the external dependency github.com/vultisig/verifier/types and should not be changed to "fee" singular, as this is how it's defined in the external package.

Learnt from: RaghavSood
PR: #36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path github.com/vultisig/verifier/plugin refers to an external dependency that provides the plugin interface, and should not be changed to github.com/vultisig/plugin/plugin as these are distinct packages with different purposes.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/constraints.go:3-3
Timestamp: 2025-07-04T10:50:42.832Z
Learning: For the vultisig/plugin project, use "fee" (singular) throughout the codebase wherever possible, not "fees" (plural). This applies to all naming conventions including plugin types, variable names, function names, comments, and documentation.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

Learnt from: johnnyluo
PR: #108
File: Dockerfile.Payroll.server:14-19
Timestamp: 2025-07-02T04:55:36.331Z
Learning: In the vultisig/plugin repository, the team maintains both the main repository and the go-wrappers dependency repository, so they are comfortable downloading from the master branch rather than pinning to specific commits.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). It contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

🧬 Code Graph Analysis (3)
cmd/fees/config/config.go (1)
api/server_config.go (1)
  • ServerConfig (9-14)
plugin/fees/fees.go (1)
internal/keysign/signer.go (1)
  • Signer (27-32)
cmd/fees/worker/main.go (6)
cmd/fees/config/config.go (1)
  • GetConfigure (36-43)
internal/keysign/signer.go (2)
  • NewSigner (34-51)
  • Emitter (23-25)
internal/keysign/api_emitter.go (1)
  • NewVerifierEmitter (12-21)
internal/keysign/plugin_emitter.go (1)
  • NewPluginEmitter (19-25)
internal/tasks/tasks.go (2)
  • TypeKeySignDKLS (13-13)
  • QUEUE_NAME (9-9)
plugin/fees/fees.go (1)
  • NewFeePlugin (50-110)
💤 Files with no reviewable changes (2)
  • api/server_config.go
  • cmd/fees/server/config.go
✅ Files skipped from review due to trivial changes (1)
  • go.mod
🚧 Files skipped from review as they are similar to previous changes (1)
  • plugin/fees/transaction.go
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: garry-sharp
PR: vultisig/plugin#117
File: plugin/fees/constraints.go:3-3
Timestamp: 2025-07-04T10:50:42.832Z
Learning: For the vultisig/plugin project, use "fee" (singular) throughout the codebase wherever possible, not "fees" (plural). This applies to all naming conventions including plugin types, variable names, function names, comments, and documentation.
Learnt from: garry-sharp
PR: vultisig/plugin#117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.
fee.server.example.json (4)

Learnt from: RaghavSood
PR: #36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path github.com/vultisig/verifier/plugin refers to an external dependency that provides the plugin interface, and should not be changed to github.com/vultisig/plugin/plugin as these are distinct packages with different purposes.

Learnt from: garry-sharp
PR: #113
File: docker-compose.yaml:80-81
Timestamp: 2025-07-03T15:55:04.605Z
Learning: For docker-compose.yaml files used for local development builds, hardcoded database credentials like "myuser:mypassword" in DATABASE_DSN environment variables are acceptable since they're only used locally and not exposed to external networks.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

Learnt from: webpiratt
PR: #127
File: internal/keysign/api_emitter.go:12-21
Timestamp: 2025-07-19T11:09:26.168Z
Learning: In the vultisig/plugin codebase, URL formatting and validation for endpoints like the verifier emitter is handled at the configuration level, so additional URL validation in individual functions is not needed.

cmd/fees/config/config.go (5)

Learnt from: RaghavSood
PR: #36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path github.com/vultisig/verifier/plugin refers to an external dependency that provides the plugin interface, and should not be changed to github.com/vultisig/plugin/plugin as these are distinct packages with different purposes.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.

Learnt from: webpiratt
PR: #126
File: plugin/fees/policy.go:26-26
Timestamp: 2025-07-10T20:41:44.025Z
Learning: The constant vtypes.PluginVultisigFees_feee in the vultisig/plugin project is correctly spelled with "feee" (including the extra 'e'). This is the actual constant name defined in the external dependency github.com/vultisig/verifier/types and should not be changed to "fee" singular, as this is how it's defined in the external package.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). It contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

cmd/fees/server/main.go (7)

Learnt from: RaghavSood
PR: #36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path github.com/vultisig/verifier/plugin refers to an external dependency that provides the plugin interface, and should not be changed to github.com/vultisig/plugin/plugin as these are distinct packages with different purposes.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.

Learnt from: webpiratt
PR: #126
File: plugin/fees/policy.go:26-26
Timestamp: 2025-07-10T20:41:44.025Z
Learning: The constant vtypes.PluginVultisigFees_feee in the vultisig/plugin project is correctly spelled with "feee" (including the extra 'e'). This is the actual constant name defined in the external dependency github.com/vultisig/verifier/types and should not be changed to "fee" singular, as this is how it's defined in the external package.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/constraints.go:3-3
Timestamp: 2025-07-04T10:50:42.832Z
Learning: For the vultisig/plugin project, use "fee" (singular) throughout the codebase wherever possible, not "fees" (plural). This applies to all naming conventions including plugin types, variable names, function names, comments, and documentation.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). It contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

Learnt from: RaghavSood
PR: #75
File: storage/postgres/db_plugin.go:25-40
Timestamp: 2025-05-30T02:44:31.711Z
Learning: The embed.FS directive //go:embed migrations/plugin/*.sql in storage/postgres/db_plugin.go correctly embeds plugin migration files that exist in the storage/postgres/migrations/plugin/ directory. The path "migrations/plugin" passed to goose.Up() references the embedded filesystem structure, not the physical file system path.

fee.worker.example.json (2)

Learnt from: RaghavSood
PR: #36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path github.com/vultisig/verifier/plugin refers to an external dependency that provides the plugin interface, and should not be changed to github.com/vultisig/plugin/plugin as these are distinct packages with different purposes.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

plugin/fees/fees.go (9)

Learnt from: webpiratt
PR: #126
File: plugin/fees/policy.go:26-26
Timestamp: 2025-07-10T20:41:44.025Z
Learning: The constant vtypes.PluginVultisigFees_feee in the vultisig/plugin project is correctly spelled with "feee" (including the extra 'e'). This is the actual constant name defined in the external dependency github.com/vultisig/verifier/types and should not be changed to "fee" singular, as this is how it's defined in the external package.

Learnt from: RaghavSood
PR: #36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path github.com/vultisig/verifier/plugin refers to an external dependency that provides the plugin interface, and should not be changed to github.com/vultisig/plugin/plugin as these are distinct packages with different purposes.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/constraints.go:3-3
Timestamp: 2025-07-04T10:50:42.832Z
Learning: For the vultisig/plugin project, use "fee" (singular) throughout the codebase wherever possible, not "fees" (plural). This applies to all naming conventions including plugin types, variable names, function names, comments, and documentation.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.

Learnt from: webpiratt
PR: #96
File: plugin/payroll/transaction.go:0-0
Timestamp: 2025-06-18T18:23:20.077Z
Learning: In the payroll plugin (plugin/payroll/transaction.go), signRequest.Transaction is stored with the "0x" prefix, making it compatible with gcommon.FromHex which requires 0x-prefixed hex strings.

Learnt from: johnnyluo
PR: #108
File: Dockerfile.Payroll.server:14-19
Timestamp: 2025-07-02T04:55:36.331Z
Learning: In the vultisig/plugin repository, the team maintains both the main repository and the go-wrappers dependency repository, so they are comfortable downloading from the master branch rather than pinning to specific commits.

Learnt from: webpiratt
PR: #96
File: plugin/payroll/transaction.go:43-44
Timestamp: 2025-06-18T18:22:06.358Z
Learning: In the vultisig/plugin codebase, the hardcoded ethereumEvmChainID = big.NewInt(1) in plugin/payroll/transaction.go is intentional for the current implementation phase. The team is implementing ETH first, with plans to add other EVM chains later. The functions/methods are already designed to work with all EVM chains.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). It contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

cmd/fees/worker/main.go (7)

Learnt from: webpiratt
PR: #126
File: plugin/fees/policy.go:26-26
Timestamp: 2025-07-10T20:41:44.025Z
Learning: The constant vtypes.PluginVultisigFees_feee in the vultisig/plugin project is correctly spelled with "feee" (including the extra 'e'). This is the actual constant name defined in the external dependency github.com/vultisig/verifier/types and should not be changed to "fee" singular, as this is how it's defined in the external package.

Learnt from: RaghavSood
PR: #36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path github.com/vultisig/verifier/plugin refers to an external dependency that provides the plugin interface, and should not be changed to github.com/vultisig/plugin/plugin as these are distinct packages with different purposes.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/constraints.go:3-3
Timestamp: 2025-07-04T10:50:42.832Z
Learning: For the vultisig/plugin project, use "fee" (singular) throughout the codebase wherever possible, not "fees" (plural). This applies to all naming conventions including plugin types, variable names, function names, comments, and documentation.

Learnt from: garry-sharp
PR: #117
File: plugin/fees/policy.go:46-47
Timestamp: 2025-07-04T10:47:47.927Z
Learning: For the fee plugin in plugin/fees/policy.go, the resource validation and recipe specification are intentionally configured to accept only USDC transfers ("ethereum.usdc.transfer"), not general ERC20 transfers ("ethereum.erc20.transfer"), as fees are only collected in USDC.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config from the external package "github.com/vultisig/verifier/vault_config" in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). The vault_config.Config struct contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

Learnt from: johnnyluo
PR: #108
File: Dockerfile.Payroll.server:14-19
Timestamp: 2025-07-02T04:55:36.331Z
Learning: In the vultisig/plugin repository, the team maintains both the main repository and the go-wrappers dependency repository, so they are comfortable downloading from the master branch rather than pinning to specific commits.

Learnt from: johnnyluo
PR: #108
File: cmd/payroll/worker/main.go:84-84
Timestamp: 2025-07-02T04:58:30.139Z
Learning: VaultServiceConfig is defined as a field of type vault_config.Config in worker configuration structs across the vultisig/plugin codebase (cmd/payroll/worker/config.go, cmd/fees/worker/config.go, cmd/dca/worker/config.go). It contains an EncryptionSecret field that can be accessed via cfg.VaultServiceConfig.EncryptionSecret.

🧬 Code Graph Analysis (3)
cmd/fees/config/config.go (1)
api/server_config.go (1)
  • ServerConfig (9-14)
plugin/fees/fees.go (1)
internal/keysign/signer.go (1)
  • Signer (27-32)
cmd/fees/worker/main.go (6)
cmd/fees/config/config.go (1)
  • GetConfigure (36-43)
internal/keysign/signer.go (2)
  • NewSigner (34-51)
  • Emitter (23-25)
internal/keysign/api_emitter.go (1)
  • NewVerifierEmitter (12-21)
internal/keysign/plugin_emitter.go (1)
  • NewPluginEmitter (19-25)
internal/tasks/tasks.go (2)
  • TypeKeySignDKLS (13-13)
  • QUEUE_NAME (9-9)
plugin/fees/fees.go (1)
  • NewFeePlugin (50-110)
⏰ 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). (2)
  • GitHub Check: build
  • GitHub Check: migration-test
🔇 Additional comments (15)
cmd/fees/config/config.go (1)

14-18: Well-structured verifier configuration.

The new verifier struct properly encapsulates the verifier-related configuration with appropriate mapstructure tags for proper unmarshaling from configuration files.

fee.server.example.json (1)

20-20: Block storage host updated for containerized environment.

The host update from localhost to minio-plugin:9000 is appropriate for containerized deployments.

cmd/fees/server/main.go (4)

16-16: Configuration package import properly aliased.

The import alias feeconfig clearly distinguishes the fee-specific configuration from other config packages.


25-25: Configuration loading updated correctly.

The change from local GetConfigure() to feeconfig.GetConfigure() properly uses the centralized configuration loader.


84-84: Signer parameter correctly set to nil for server.

Since the server doesn't perform signing operations, passing nil for the signer parameter is appropriate. The actual signer is used in the worker component.


93-93: Verifier URL reference updated correctly.

The change from cfg.Server.VerifierUrl to cfg.Verifier.URL properly uses the new configuration structure.

fee.worker.example.json (1)

2-6: Complete verifier configuration with all required fields.

The verifier configuration section includes all necessary fields (url, token, party_prefix) that align with the verifier struct definition in cmd/fees/config/config.go. This supports the signer integration in the worker component.

cmd/fees/worker/main.go (5)

11-11: Required imports added for signer integration.

The new imports for keysign, relay, and feeconfig are necessary for the signer functionality integration.

Also applies to: 15-15, 17-17


26-26: Configuration loading updated consistently.

The change to use feeconfig.GetConfigure() maintains consistency with the server component.


98-109: Signer properly configured with emitters and party prefixes.

The signer configuration correctly includes:

  • Logger with appropriate field
  • Relay client from vault service config
  • Two emitters: verifier and plugin emitters
  • Party prefixes from both verifier and vault service config

This setup enables proper key signing functionality for the fee plugin.


113-113: Signer correctly passed to fee plugin constructor.

The signer instance is properly passed to the NewFeePlugin constructor, enabling signing capabilities in the worker component.


122-122: Verifier URL reference updated correctly.

The change from the removed cfg.Server.VerifierUrl to cfg.Verifier.URL properly uses the new configuration structure.

plugin/fees/fees.go (3)

12-12: LGTM! Import addition is necessary for the new signer functionality.

The import is correctly added to support the new *keysign.Signer type being integrated into the plugin.


100-100: LGTM! Field assignment is correct.

The signer field is properly assigned in the constructor following the established pattern.


38-38: Signer field usage confirmed

The signer field is actively used in the fee plugin’s transaction flow:

  • plugin/fees/transaction.go: calls fp.signer.Sign(ctx, req) for signing operations.

No further action required—approving the addition.

Comment thread fee.server.example.json
Comment on lines +2 to +4
"verifier": {
"url": "verifier:8080"
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Verifier configuration restructured consistently.

The new verifier configuration section aligns with the code changes in cmd/fees/config/config.go. However, the configuration is missing the token and party_prefix fields that are defined in the verifier struct.

Apply this diff to complete the verifier configuration:

     "verifier": {
-        "url": "verifier:8080"
+        "url": "verifier:8080",
+        "token": "example-token",
+        "party_prefix": "server"
     },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"verifier": {
"url": "verifier:8080"
},
"verifier": {
"url": "verifier:8080",
"token": "example-token",
"party_prefix": "server"
},
🤖 Prompt for AI Agents
In fee.server.example.json around lines 2 to 4, the verifier configuration is
missing the token and party_prefix fields required by the verifier struct in
cmd/fees/config/config.go. Add the token and party_prefix fields with
appropriate example values to the verifier section to align the configuration
with the updated struct definition.

Comment thread plugin/fees/fees.go
}

func NewFeePlugin(db storage.DatabaseStorage,
signer *keysign.Signer,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add nil validation for the signer parameter.

Consider adding validation to ensure the signer parameter is not nil, similar to the existing validation for db, vaultStorage, and other critical dependencies.

+	if signer == nil {
+		return nil, fmt.Errorf("signer cannot be nil")
+	}
+
 	if db == nil {
 		return nil, fmt.Errorf("database storage cannot be nil")
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
signer *keysign.Signer,
if signer == nil {
return nil, fmt.Errorf("signer cannot be nil")
}
if db == nil {
return nil, fmt.Errorf("database storage cannot be nil")
}
🤖 Prompt for AI Agents
In plugin/fees/fees.go at line 51, add a nil check for the signer parameter
similar to the existing validations for db and vaultStorage. If signer is nil,
return an appropriate error early to prevent potential nil pointer dereferences
later in the code.

Comment thread cmd/fees/config/config.go
Comment on lines +14 to +18
type verifier struct {
URL string `mapstructure:"url"`
Token string `mapstructure:"token"`
PartyPrefix string `mapstructure:"party_prefix"`
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel like this overlaps with another verifier config - non-blocking, largely up to you on how you want to handle this, but maybe worth checking later

@RaghavSood RaghavSood left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@garry-sharp
garry-sharp merged commit f8680a8 into main Jul 23, 2025
3 checks passed
@garry-sharp
garry-sharp deleted the small-bug-fixes-fees branch July 23, 2025 05:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants