🔧 Config added for fees, makefile added to run dev mode (hot reloadin…#113
Conversation
…g or plugins). Fee worker has been added here to enable hot reloading.
WalkthroughThis change introduces new Dockerfiles, Docker Compose configurations, and supporting files to add "fee-worker" and "fee-server" services alongside development and production workflows. It updates configuration files, refines database and Redis service naming, and enhances error handling in the worker's main Go entrypoint. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Docker as Docker Engine
participant FeeWorker as Fee Worker Container
participant FeeServer as Fee Server Container
participant DB as plugin-db
participant Redis as redis-fees
Dev->>Docker: docker-compose -f docker-compose.yaml up
Docker->>FeeWorker: Build and start container
Docker->>FeeServer: Build and start container
FeeWorker->>DB: Connect to vultisig-fee database
FeeWorker->>Redis: Connect to redis-fees
FeeServer->>DB: Connect to vultisig-fee database
FeeServer->>Redis: Connect to redis-fees
Possibly related PRs
Suggested reviewers
Poem
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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces configuration and service setup for the fees plugin alongside updating existing payroll services to share a single plugin-db instance, and adds Makefile targets for dev mode (hot reloading).
- Updated all payroll DSNs to point at a shared
plugin-dbinstead ofpayroll-db - Added fee plugin example configs (
.jsonand.yml), Docker Compose services, and Dockerfiles (dev+prod) - Introduced
docker-compose.dev.yamland new Makefile targets (up-dev,build-dev, etc.)
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| payroll.worker.example.json | Changed DSN to use plugin-db and correct database name |
| payroll.server.example.json | Updated DSN to use plugin-db |
| fee.worker.example.json | Added example JSON config for the fees worker plugin |
| fee.server.example.json | Added example JSON config for the fees server plugin |
| etc/vultisig/fee.yml | New fee plugin YAML config for gas and RPC settings |
| docker-compose.yaml | Renamed payroll-db to plugin-db, added redis-fees and fee-worker service |
| docker-compose.dev.yaml | New dev compose file for hot-reloading the fee worker |
| Makefile | Added up-dev, down-dev, build, and build-dev targets |
| Dockerfile.Fee.worker.dev | New Dockerfile for fee worker with hot reload via Air |
| Dockerfile.Fee.worker | New multi-stage Dockerfile for fee worker (production) |
Comments suppressed due to low confidence (1)
docker-compose.dev.yaml:13
- [nitpick] This volume mount may create a directory rather than mounting the intended host file. Verify that you’re specifying
<host_path>:<container_path>and that the host path exists.
- /app/build-errors.log
| "encryption_secret": "test123" | ||
| }, | ||
| "database": { | ||
| "dsn": "postgres://myuser:mypassword@localhost:5431/vultisig-fee?sslmode=disable" |
There was a problem hiding this comment.
[nitpick] This server example still points at localhost; to mirror the worker and other services, consider using the plugin-db host here as well.
| "dsn": "postgres://myuser:mypassword@localhost:5431/vultisig-fee?sslmode=disable" | |
| "dsn": "postgres://myuser:mypassword@plugin-db:5431/vultisig-fee?sslmode=disable" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
Makefile (1)
1-3: DYLD_LIBRARY variable is misspelled; should be DYLD_LIBRARY_PATH
DYLD_LIBRARYis ignored by the macOS loader. ExportDYLD_LIBRARY_PATH(or drop entirely) so the dynamic linker can locate DKLS libs.-DYLD_LIBRARY=../go-wrappers/includes/darwin/:$LD_LIBRARY_PATH +DYLD_LIBRARY_PATH=../go-wrappers/includes/darwin/:${DYLD_LIBRARY_PATH:-}
🧹 Nitpick comments (11)
payroll.worker.example.json (1)
6-8: Replace placeholderencryption_secretbefore shipping
"encryption_secret": "test123"is fine for a template, but make sure CI / production artefacts never ship with this value.fee.server.example.json (1)
5-5: Template secret must be rotated
Don’t forget to override"encryption_secret": "test123"in any non-local deployment.fee.worker.example.json (1)
11-11: Remember to rotate the placeholderencryption_secret
Same caveat as other example configs.payroll.server.example.json (1)
5-5: Rotateencryption_secretbefore production rollout
Same note as other example configs.Makefile (2)
4-6: Mark targets as phony to avoid name clashesAdd a
.PHONYblock so make doesn’t treat existing files namedbuild,up-dev, etc. as up-to-date targets.+.PHONY: up up-dev down down-dev build build-dev dump-schemaAlso applies to: 13-21
7-9: DRY: extract compose file list into a variableFour targets repeat the same
-f docker-compose.yaml -f docker-compose.dev.yaml. Use a variable to avoid drift:COMPOSE_DEV = -f docker-compose.yaml -f docker-compose.dev.yaml up-dev: @docker compose $(COMPOSE_DEV) up -d --remove-orphans ...Also applies to: 16-21
docker-compose.yaml (1)
82-82: Remove trailing whitespace (YAMLlint error)
Line 82 contains a stray space after the port mapping.- - "8081:8080" + - "8081:8080"Dockerfile.Fee.worker.dev (1)
14-20: Clean up downloaded tarball to shrink imageThe DKLS archive and extracted folder remain, adding ~40 MB. Compress the RUN chain and delete artifacts:
-RUN wget https://github.com/vultisig/go-wrappers/archive/refs/heads/master.tar.gz -RUN tar -xzf master.tar.gz && \ - cd go-wrappers-master && \ - mkdir -p /usr/local/lib/dkls && \ - cp --recursive includes /usr/local/lib/dkls +RUN wget -q https://github.com/vultisig/go-wrappers/archive/refs/heads/master.tar.gz && \ + tar -xzf master.tar.gz && \ + cd go-wrappers-master && \ + mkdir -p /usr/local/lib/dkls && \ + cp -r includes /usr/local/lib/dkls && \ + cd .. && rm -rf master.tar.gz go-wrappers-masterdocker-compose.dev.yaml (1)
36-39: Fix YAML lint errors (trailing spaces & EOF newline)
Remove trailing spaces on line 36 and add a newline at EOF to satisfy linters.Dockerfile.Fee.worker (2)
14-19: Clean up DKLS download in builder stageDelete the archive and extracted dir to keep the builder layer smaller:
-RUN wget https://github.com/vultisig/go-wrappers/archive/refs/heads/master.tar.gz -RUN tar -xzf master.tar.gz && \ +RUN wget -q https://github.com/vultisig/go-wrappers/archive/refs/heads/master.tar.gz && \ + tar -xzf master.tar.gz && \ cd go-wrappers-master && \ mkdir -p /usr/local/lib/dkls && \ - cp --recursive includes /usr/local/lib/dkls + cp -r includes /usr/local/lib/dkls && \ + cd .. && rm -rf master.tar.gz go-wrappers-master
26-32: Runtime optimisation: use a slimmer base imageConsider
ubuntu:24.04-minimalor a distroless image to cut size and CVE surface area; you only needglibcand the native libs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Dockerfile.Fee.worker(1 hunks)Dockerfile.Fee.worker.dev(1 hunks)Makefile(1 hunks)docker-compose.dev.yaml(1 hunks)docker-compose.yaml(4 hunks)etc/vultisig/fee.yml(1 hunks)fee.server.example.json(1 hunks)fee.worker.example.json(1 hunks)payroll.server.example.json(1 hunks)payroll.worker.example.json(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
payroll.server.example.json (2)
Learnt from: johnnyluo
PR: vultisig/plugin#108
File: payroll.worker.example.json:8-8
Timestamp: 2025-07-02T04:57:55.842Z
Learning: Example configuration files (files with ".example." in the name) for local testing purposes are allowed to have weak or placeholder encryption secrets like "test123" since they are templates meant to be replaced with secure values in production deployments.
Learnt from: RaghavSood
PR: vultisig/plugin#70
File: .github/workflows/migration-test.yml:14-17
Timestamp: 2025-05-27T04:28:20.414Z
Learning: In GitHub Actions workflows for testing, hardcoded database credentials are acceptable when using ephemeral test services (like PostgreSQL containers) that only exist during the CI run, as there's no security risk with temporary test environments.
fee.worker.example.json (3)
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.
Learnt from: johnnyluo
PR: vultisig/plugin#108
File: payroll.worker.example.json:8-8
Timestamp: 2025-07-02T04:57:55.842Z
Learning: Example configuration files (files with ".example." in the name) for local testing purposes are allowed to have weak or placeholder encryption secrets like "test123" since they are templates meant to be replaced with secure values in production deployments.
payroll.worker.example.json (4)
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.
Learnt from: webpiratt
PR: vultisig/plugin#96
File: plugin/payroll/transaction.go:178-183
Timestamp: 2025-06-18T18:28:19.759Z
Learning: In the payroll plugin, the Hash field in PluginKeysignRequest is intentionally set to the unsigned transaction hex (same as Message field) because computing a hash with empty V,R,S signature fields doesn't make sense, and it's documented as "not on-chain hash without signature". This is a deliberate placeholder approach.
Learnt from: johnnyluo
PR: vultisig/plugin#108
File: payroll.worker.example.json:8-8
Timestamp: 2025-07-02T04:57:55.842Z
Learning: Example configuration files (files with ".example." in the name) for local testing purposes are allowed to have weak or placeholder encryption secrets like "test123" since they are templates meant to be replaced with secure values in production deployments.
etc/vultisig/fee.yml (1)
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.
fee.server.example.json (2)
Learnt from: johnnyluo
PR: vultisig/plugin#108
File: payroll.worker.example.json:8-8
Timestamp: 2025-07-02T04:57:55.842Z
Learning: Example configuration files (files with ".example." in the name) for local testing purposes are allowed to have weak or placeholder encryption secrets like "test123" since they are templates meant to be replaced with secure values in production deployments.
Learnt from: RaghavSood
PR: vultisig/plugin#70
File: .github/workflows/migration-test.yml:14-17
Timestamp: 2025-05-27T04:28:20.414Z
Learning: In GitHub Actions workflows for testing, hardcoded database credentials are acceptable when using ephemeral test services (like PostgreSQL containers) that only exist during the CI run, as there's no security risk with temporary test environments.
docker-compose.yaml (1)
Learnt from: RaghavSood
PR: vultisig/plugin#70
File: .github/workflows/migration-test.yml:14-17
Timestamp: 2025-05-27T04:28:20.414Z
Learning: In GitHub Actions workflows for testing, hardcoded database credentials are acceptable when using ephemeral test services (like PostgreSQL containers) that only exist during the CI run, as there's no security risk with temporary test environments.
🪛 Checkov (3.2.334)
payroll.server.example.json
[MEDIUM] 8-9: Basic Auth Credentials
(CKV_SECRET_4)
fee.worker.example.json
[MEDIUM] 31-32: Basic Auth Credentials
(CKV_SECRET_4)
payroll.worker.example.json
[MEDIUM] 28-29: Basic Auth Credentials
(CKV_SECRET_4)
fee.server.example.json
[MEDIUM] 8-9: Basic Auth Credentials
(CKV_SECRET_4)
docker-compose.dev.yaml
[MEDIUM] 10-11: Basic Auth Credentials
(CKV_SECRET_4)
docker-compose.yaml
[MEDIUM] 80-81: Basic Auth Credentials
(CKV_SECRET_4)
🪛 YAMLlint (1.37.1)
docker-compose.dev.yaml
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 39-39: no new line character at the end of file
(new-line-at-end-of-file)
docker-compose.yaml
[error] 82-82: trailing spaces
(trailing-spaces)
🔇 Additional comments (9)
payroll.worker.example.json (2)
10-14: Redis port (6378) is non-standard – verify it matches the Compose service
Every other service uses the default6379. Using6378will break the connection unless a custom-port Redis container is started.
28-28: Confirm theplugin-db:5432endpoint exists in the target network
The DSN was updated, but the Compose file must expose the DB on 5432 and under theplugin-dbhostname for this to work.fee.server.example.json (1)
12-14: Redis port (6377) deviates from the default
Ensure the dev Compose stack exposes Redis on6377; otherwise, align the port to6379.fee.worker.example.json (2)
14-17: Validate Redis host/port availability
Container must be namedredis-feesand listen on6379; double-check docker-compose.*.yaml.
31-32: DSN relies onplugin-db:5432; confirm network alias exists
Mismatch here will prevent the worker from starting.payroll.server.example.json (1)
8-9: DSN update LGTM – consistent with newplugin-dbservice
No action needed if Compose exposes the DB on 5432.docker-compose.yaml (1)
131-138: External Docker network must exist
shared_networkisexternal: true;docker compose upfails if the network hasn’t been created. Add a Make target or note in README:docker network create shared_networkDockerfile.Fee.worker.dev (1)
2-2: Verify availability ofgolang:1.24As of today, Docker Hub only lists Go 1.23. Pinning to a non-existent tag will break CI/CD.
Dockerfile.Fee.worker (1)
2-2: Confirmgolang:1.24tag existsSame concern as dev Dockerfile—pin to a valid tag or build will fail.
| gas: | ||
| limit_multiplier: 300 # 300% of estimated gas | ||
| price_multiplier: 3 # 300% of suggested gas price |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent multipliers – 300 vs 3 both claim “300 %”
limit_multiplier is set to 300 (3 ×), while price_multiplier is 3 (also 3 ×). Choose a single scale (either 3 or 300) to avoid mis-reading at runtime.
-limit_multiplier: 300 # 300% of estimated gas
-price_multiplier: 3 # 300% of suggested gas price
+limit_multiplier: 3 # 300% of estimated gas
+price_multiplier: 3 # 300% of suggested gas price📝 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.
| gas: | |
| limit_multiplier: 300 # 300% of estimated gas | |
| price_multiplier: 3 # 300% of suggested gas price | |
| gas: | |
| limit_multiplier: 3 # 300% of estimated gas | |
| price_multiplier: 3 # 300% of suggested gas price |
🤖 Prompt for AI Agents
In etc/vultisig/fee.yml around lines 5 to 7, the gas multipliers are
inconsistent: limit_multiplier is set to 300 (300%) while price_multiplier is
set to 3 (3×), both commented as 300%. Choose one scale for both multipliers,
either 3 or 300, and update the values and comments accordingly to ensure
consistent interpretation at runtime.
| DATABASE_DSN: postgres://myuser:mypassword@plugin-db:5432/vultisig-fee?sslmode=disable | ||
| volumes: |
There was a problem hiding this comment.
Credentials in DSN leak secrets
Same issue as production compose—move myuser:mypassword into secrets or an env-file.
🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 10-11: Basic Auth Credentials
(CKV_SECRET_4)
🤖 Prompt for AI Agents
In docker-compose.dev.yaml around lines 10 to 11, the DATABASE_DSN environment
variable contains hardcoded credentials which leak secrets. To fix this, remove
the username and password from the DSN string and instead reference them
securely via Docker secrets or an environment file. Update the compose file to
load these credentials from the secrets or env-file and construct the DSN
dynamically or via environment variables without exposing sensitive information
directly in the compose file.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Dockerfile.Fee.server (1)
14-18: Same supply-chain & reproducibility concern as the dev DockerfilePin the
go-wrappersarchive to a commit/tag and verify its checksum (see earlier suggestion).
🧹 Nitpick comments (6)
fee.server.example.json (1)
17-23: Block storage host inconsistency with worker configuration.The block storage host uses
localhost:9100while the worker configuration usesminio-plugin:9000. For consistency and proper containerized networking, consider using the same service name and port.- "host": "http://localhost:9100", + "host": "http://minio-plugin:9000",Dockerfile.Fee.server.dev (1)
26-34: Use a static.air.tomlinstead of a long echo chainCommitting the file (or templating it) improves readability and avoids brittle multilines in the Dockerfile.
docker-compose.yaml (2)
82-82: Remove trailing whitespace to satisfy linters- - "8081:8080" + - "8081:8080" … - - "8082:8080" + - "8082:8080"Also applies to: 125-125
113-146: Reduce duplication with YAML anchors
REDIS_*,BLOCK_STORAGE_HOST, andDATABASE_DSNare repeated four times.
Consider anchors & aliases (or an env-file) to keep the file DRY and ease future edits.docker-compose.dev.yaml (1)
34-34: Lint errors: trailing space & missing newline- minio_data: + minio_data: +Also applies to: 37-37
Dockerfile.Fee.server (1)
25-32: Large runtime image & runs as rootMoving to a slim base (e.g.
gcr.io/distroless/cc) and adding a non-root user will cut image size and harden security:-FROM ubuntu:24.04 +FROM gcr.io/distroless/cc … -# Run the application -CMD ["./server"] +USER 10001:10001 # non-privileged +ENTRYPOINT ["/app/server"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Dockerfile.Fee.server(1 hunks)Dockerfile.Fee.server.dev(1 hunks)Makefile(1 hunks)cmd/fees/worker/main.go(1 hunks)docker-compose.dev.yaml(1 hunks)docker-compose.yaml(4 hunks)etc/vultisig/fee.yml(1 hunks)fee.server.example.json(1 hunks)fee.worker.example.json(1 hunks)fee_config.example.json(0 hunks)init-scripts/01_create_vultisig_plugin.sql(1 hunks)
💤 Files with no reviewable changes (1)
- fee_config.example.json
🚧 Files skipped from review as they are similar to previous changes (1)
- etc/vultisig/fee.yml
🧰 Additional context used
🧠 Learnings (6)
cmd/fees/worker/main.go (1)
Learnt from: RaghavSood
PR: vultisig/plugin#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.
docker-compose.dev.yaml (2)
Learnt from: garry-sharp
PR: vultisig/plugin#113
File: docker-compose.yaml:80-81
Timestamp: 2025-07-03T15:55:04.565Z
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: RaghavSood
PR: vultisig/plugin#70
File: .github/workflows/migration-test.yml:14-17
Timestamp: 2025-05-27T04:28:20.414Z
Learning: In GitHub Actions workflows for testing, hardcoded database credentials are acceptable when using ephemeral test services (like PostgreSQL containers) that only exist during the CI run, as there's no security risk with temporary test environments.
docker-compose.yaml (3)
Learnt from: garry-sharp
PR: vultisig/plugin#113
File: docker-compose.yaml:80-81
Timestamp: 2025-07-03T15:55:04.565Z
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: RaghavSood
PR: vultisig/plugin#70
File: .github/workflows/migration-test.yml:14-17
Timestamp: 2025-05-27T04:28:20.414Z
Learning: In GitHub Actions workflows for testing, hardcoded database credentials are acceptable when using ephemeral test services (like PostgreSQL containers) that only exist during the CI run, as there's no security risk with temporary test environments.
Learnt from: johnnyluo
PR: vultisig/plugin#108
File: payroll.worker.example.json:8-8
Timestamp: 2025-07-02T04:57:55.842Z
Learning: Example configuration files (files with ".example." in the name) for local testing purposes are allowed to have weak or placeholder encryption secrets like "test123" since they are templates meant to be replaced with secure values in production deployments.
Makefile (1)
Learnt from: garry-sharp
PR: vultisig/plugin#113
File: docker-compose.yaml:80-81
Timestamp: 2025-07-03T15:55:04.565Z
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.
fee.server.example.json (4)
Learnt from: garry-sharp
PR: vultisig/plugin#113
File: docker-compose.yaml:80-81
Timestamp: 2025-07-03T15:55:04.565Z
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: RaghavSood
PR: vultisig/plugin#70
File: .github/workflows/migration-test.yml:14-17
Timestamp: 2025-05-27T04:28:20.414Z
Learning: In GitHub Actions workflows for testing, hardcoded database credentials are acceptable when using ephemeral test services (like PostgreSQL containers) that only exist during the CI run, as there's no security risk with temporary test environments.
Learnt from: johnnyluo
PR: vultisig/plugin#108
File: payroll.worker.example.json:8-8
Timestamp: 2025-07-02T04:57:55.842Z
Learning: Example configuration files (files with ".example." in the name) for local testing purposes are allowed to have weak or placeholder encryption secrets like "test123" since they are templates meant to be replaced with secure values in production deployments.
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.
fee.worker.example.json (5)
Learnt from: garry-sharp
PR: vultisig/plugin#113
File: fee.worker.example.json:9-9
Timestamp: 2025-07-03T15:54:18.752Z
Learning: In configuration files, local_party_prefix values can contain hexadecimal digit patterns like "feee" which are intentional, not typos.
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.
Learnt from: garry-sharp
PR: vultisig/plugin#113
File: docker-compose.yaml:80-81
Timestamp: 2025-07-03T15:55:04.565Z
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: vultisig/plugin#108
File: payroll.worker.example.json:8-8
Timestamp: 2025-07-02T04:57:55.842Z
Learning: Example configuration files (files with ".example." in the name) for local testing purposes are allowed to have weak or placeholder encryption secrets like "test123" since they are templates meant to be replaced with secure values in production deployments.
🪛 YAMLlint (1.37.1)
docker-compose.dev.yaml
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 37-37: no new line character at the end of file
(new-line-at-end-of-file)
docker-compose.yaml
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 125-125: trailing spaces
(trailing-spaces)
🪛 Checkov (3.2.334)
docker-compose.yaml
[MEDIUM] 80-81: Basic Auth Credentials
(CKV_SECRET_4)
fee.server.example.json
[MEDIUM] 9-10: Basic Auth Credentials
(CKV_SECRET_4)
fee.worker.example.json
[MEDIUM] 31-32: Basic Auth Credentials
(CKV_SECRET_4)
🪛 checkmake (0.2.2)
Makefile
[warning] 6-6: Missing required phony target "all"
(minphony)
[warning] 6-6: Missing required phony target "clean"
(minphony)
[warning] 6-6: Missing required phony target "test"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (7)
init-scripts/01_create_vultisig_plugin.sql (1)
1-3: LGTM! Database creation script is well-structured.The SQL script correctly creates the required databases for the fee plugin infrastructure. The naming convention is consistent and aligns with the service architecture introduced in this PR.
cmd/fees/worker/main.go (1)
59-61: Excellent error handling improvement!Adding proper error handling for the
postgres.NewPostgresBackendcall prevents potential nil pointer dereferences and provides clear error messages for debugging. The error wrapping follows Go best practices.fee.worker.example.json (1)
1-33: Configuration file is well-structured and complete.The JSON configuration covers all necessary components for the fee worker service including server endpoints, vault service parameters, Redis connection, block storage, monitoring, and database configuration. The structure aligns well with the Docker Compose setup introduced in this PR.
Makefile (3)
3-3: Good addition of DYLD_LIBRARY_PATH for native library support.The new environment variable properly supports the native library requirements for the fee services, complementing the existing DYLD_LIBRARY variable.
6-6: Well-maintained .PHONY declaration.The .PHONY declaration properly includes all targets, preventing conflicts with files of the same name and ensuring targets always execute.
11-25: Excellent development workflow support.The new targets (
up-dev,down-dev,build,build-dev) provide clean separation between production and development environments using Docker Compose override files. The implementation follows consistent patterns with existing targets.Dockerfile.Fee.server.dev (1)
14-21: Un-pinned GitHub archive → unreproducible & potentially unsafe image
master.tar.gzchanges on every commit; no checksum is verified.
Pin to a commit SHA/tag and verify its digest, then delete the archive to keep the layer slim:-RUN wget https://github.com/vultisig/go-wrappers/archive/refs/heads/master.tar.gz -RUN tar -xzf master.tar.gz && \ +ARG WRAPPER_SHA=06d7c3d # example pinned commit +RUN wget -q https://github.com/vultisig/go-wrappers/archive/${WRAPPER_SHA}.tar.gz -O wrappers.tar.gz && \ + echo "2e6f… wrappers.tar.gz" | sha256sum -c - && \ + tar -xzf wrappers.tar.gz && \ cd go-wrappers-${WRAPPER_SHA} && \ mkdir -p /usr/local/lib/dkls && \ cp --recursive includes /usr/local/lib/dkls && \ - cd .. && rm -rf go-wrappers-* master.tar.gz + cd .. && rm -rf go-wrappers-* wrappers.tar.gz⛔ Skipped due to learnings
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.
| # Use the official Go image as the base image | ||
| FROM golang:1.24 |
There was a problem hiding this comment.
Pin to an existing Go release – 1.24 does not yet exist
The official images only go up to 1.22.x at the moment. Referencing a non-existent tag will break the build once the cache is bust.
Update to the latest stable (e.g. golang:1.22-alpine) or pin a specific patch version.
🤖 Prompt for AI Agents
In Dockerfile.Fee.server.dev at lines 1 to 2, the base image is set to
golang:1.24, which does not exist yet and will cause build failures. Update the
FROM line to use an existing stable Go version tag such as golang:1.22-alpine or
a specific patch version like golang:1.22.9-alpine to ensure the image is
available and the build succeeds.
| # Use the official Go image as the base image | ||
| FROM golang:1.24 as builder |
There was a problem hiding this comment.
Invalid base tag – Go 1.24 image not yet published
Switch to an existing tag (golang:1.22, etc.) or your private registry mirror.
🤖 Prompt for AI Agents
In Dockerfile.Fee.server at lines 1 to 2, the base image tag 'golang:1.24' is
invalid because it is not yet published. Change the base image tag to a valid
existing version such as 'golang:1.22' or use a tag from your private registry
mirror to ensure the image can be pulled successfully.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docker-compose.yaml (1)
84-85: Plain-text DB credentials — already discussedThis exact point was covered in the previous review and accepted for local-dev context, so nothing to change here.
Also applies to: 106-107, 127-128, 149-150
🧹 Nitpick comments (2)
docker-compose.yaml (2)
86-86: Remove trailing whitespace to satisfy YAMLlint
yaml-lintis failing on these lines due to a single blank after the last token. No functional impact, but it blocks clean CI runs and cluttersgithistory.- - "8081:8080"␠ + - "8081:8080" - - "8082:8080"␠ + - "8082:8080" - minio_data: ␠ + minio_data:Also applies to: 129-129, 162-162
117-158: Reduce copy-pasted blocks with YAML anchors / env_file
fee-serverandfee-workerrepeat the samebuild,environment,depends_on, andnetworkssections. This duplication increases maintenance surface (four DSN edits instead of one, etc.).Minimal example using anchors:
&fee-template fee-common: &fee-common build: context: . platforms: [linux/amd64] environment: REDIS_HOST: redis-fees REDIS_PORT: 6379 BLOCK_STORAGE_HOST: http://minio-plugin:9000 DATABASE_DSN: postgres://myuser:mypassword@plugin-db:5432/vultisig-fee?sslmode=disable depends_on: plugin-db: { condition: service_healthy } redis-fees: { condition: service_healthy } minio-plugin: { condition: service_started } networks: [shared_network] fee-server: <<: *fee-common build: dockerfile: Dockerfile.Fee.server ports: ["8082:8080"] fee-worker: <<: *fee-common build: dockerfile: Dockerfile.Fee.workerAlternatives: move the variables to an
env_file, or usex-extension fields (Compose v3.9+).
Not urgent, but worthwhile for long-term maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docker-compose.yaml(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
docker-compose.yaml (3)
Learnt from: garry-sharp
PR: vultisig/plugin#113
File: docker-compose.yaml:80-81
Timestamp: 2025-07-03T15:55:04.565Z
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: RaghavSood
PR: vultisig/plugin#70
File: .github/workflows/migration-test.yml:14-17
Timestamp: 2025-05-27T04:28:20.414Z
Learning: In GitHub Actions workflows for testing, hardcoded database credentials are acceptable when using ephemeral test services (like PostgreSQL containers) that only exist during the CI run, as there's no security risk with temporary test environments.
Learnt from: johnnyluo
PR: vultisig/plugin#108
File: payroll.worker.example.json:8-8
Timestamp: 2025-07-02T04:57:55.842Z
Learning: Example configuration files (files with ".example." in the name) for local testing purposes are allowed to have weak or placeholder encryption secrets like "test123" since they are templates meant to be replaced with secure values in production deployments.
🪛 Checkov (3.2.334)
docker-compose.yaml
[MEDIUM] 84-85: Basic Auth Credentials
(CKV_SECRET_4)
🪛 YAMLlint (1.37.1)
docker-compose.yaml
[error] 86-86: trailing spaces
(trailing-spaces)
[error] 129-129: trailing spaces
(trailing-spaces)
[error] 162-162: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docker-compose.yaml (1)
84-85: Plain-text DSNs acknowledged – previous discussion applies
These lines still embedmyuser:mypassword. Per the prior conversation we had in this PR, this is acceptable for local-dev only and no further action is needed.Also applies to: 106-107, 127-128, 149-150
🧹 Nitpick comments (2)
docker-compose.yaml (2)
21-47: Factor out the two Redis service blocks to cut duplication
redis-payrollandredis-feesare byte-for-byte identical apart from the host-port mapping and volume name. Collapsing the common parts into an anchor/alias (or a Compose extension field) keeps the file shorter and lowers the chance of future drift.Example sketch:
x-redis-common: &redis-common image: redis:latest networks: [shared_network] healthcheck: test: ["CMD", "redis-cli", "ping"] interval: 1s timeout: 5s retries: 3 services: redis-payroll: <<: *redis-common ports: ["6378:6379"] volumes: [redis_payroll:/data] redis-fees: <<: *redis-common ports: ["6377:6379"] volumes: [redis_fees:/data]This keeps the intent clear while avoiding copy-paste maintenance.
86-86: Strip trailing whitespace to satisfy YAML lint
yamllintflags trailing spaces on these lines. Removing them keeps CI tools quiet and avoids noisy diffs later.- - "8081:8080"␠ + - "8081:8080"(similarly for lines 129 and 162)
Also applies to: 129-129, 162-162
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docker-compose.yaml(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
docker-compose.yaml (3)
Learnt from: garry-sharp
PR: vultisig/plugin#113
File: docker-compose.yaml:80-81
Timestamp: 2025-07-03T15:55:04.565Z
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: RaghavSood
PR: vultisig/plugin#70
File: .github/workflows/migration-test.yml:14-17
Timestamp: 2025-05-27T04:28:20.414Z
Learning: In GitHub Actions workflows for testing, hardcoded database credentials are acceptable when using ephemeral test services (like PostgreSQL containers) that only exist during the CI run, as there's no security risk with temporary test environments.
Learnt from: johnnyluo
PR: vultisig/plugin#108
File: payroll.worker.example.json:8-8
Timestamp: 2025-07-02T04:57:55.842Z
Learning: Example configuration files (files with ".example." in the name) for local testing purposes are allowed to have weak or placeholder encryption secrets like "test123" since they are templates meant to be replaced with secure values in production deployments.
🪛 Checkov (3.2.334)
docker-compose.yaml
[MEDIUM] 84-85: Basic Auth Credentials
(CKV_SECRET_4)
🪛 YAMLlint (1.37.1)
docker-compose.yaml
[error] 86-86: trailing spaces
(trailing-spaces)
[error] 129-129: trailing spaces
(trailing-spaces)
[error] 162-162: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
docker-compose.yaml (1)
165-167: Verify that the external network exists beforedocker compose up
Becauseshared_networkis markedexternal: true, developers need to create it once (docker network create shared_network) or Compose will fail. Confirm this is documented in the README/Makefile so first-time contributors don’t hit a dead end.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores