chore(coprocessor): Improve usability of DatabaseURL #1082
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
goshawk-3
force-pushed
the
georgi/use-db-url
branch
from
fec7784 to
3a03ea4
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a DatabaseURL wrapper type to improve security and consistency of database URL handling across the codebase. The main purpose is to mask database credentials in logs and provide a centralized type for database URL operations.
- Adds a new
DatabaseURLtype infhevm-engine-common/src/utils.rswith password masking capabilities - Replaces
Stringtype withDatabaseURLfor all database URL fields across multiple modules - Updates logging to use
%(Display) instead of?(Debug) for sensitive config structs
Reviewed Changes
Copilot reviewed 27 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| coprocessor/fhevm-engine/fhevm-engine-common/src/utils.rs | Introduces DatabaseURL wrapper with Display/Debug impls that mask passwords |
| coprocessor/fhevm-engine/zkproof-worker/src/lib.rs | Updates Config.database_url to DatabaseURL type and adds Display impl |
| coprocessor/fhevm-engine/zkproof-worker/src/verifier.rs | Changes logging format specifier from ? to % for config |
| coprocessor/fhevm-engine/sns-worker/src/lib.rs | Updates DBConfig.url to DatabaseURL and removes custom Debug impl, changes logging |
| coprocessor/fhevm-engine/transaction-sender/src/config.rs | Updates ConfigSettings.database_url to Option<DatabaseURL> |
| coprocessor/fhevm-engine/tfhe-worker/src/daemon_cli.rs | Updates args to accept Option<DatabaseURL> |
| coprocessor/fhevm-engine/tfhe-worker/src/utils.rs | Removes db_url() helper function (now handled by DatabaseURL::default()) |
| coprocessor/fhevm-engine/test-harness/src/instance.rs | Updates DBInstance.db_url field to be public DatabaseURL type |
| coprocessor/fhevm-engine/stress-test-generator/src/utils.rs | Updates ACL contract address default value |
| Multiple test files | Updates test code to use new DatabaseURL type with .into() conversions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
goshawk-3
force-pushed
the
georgi/use-db-url
branch
from
1eaae99 to
394aac1
Compare
antoniupop
approved these changes
Nov 6, 2025
Collaborator
antoniupop
left a comment
antoniupop
left a comment
There was a problem hiding this comment.
LGTM - 2 possibly mistaken inclusions in the PR
coprocessor/fhevm-engine/stress-test-generator/data/json/minitest_002_erc20.json
Outdated
Show resolved
Hide resolved
goshawk-3
force-pushed
the
georgi/use-db-url
branch
from
394aac1 to
8ea42e4
Compare
goshawk-3
force-pushed
the
georgi/use-db-url
branch
from
3238e30 to
2b984c7
Compare
goshawk-3
added a commit
that referenced
this pull request
Nov 7, 2025
* chore(coprocessor): implement a DatabaseURL wrapper * chore(coprocessor): use DatabaseURL in sns-worker * chore(coprocessor): use DatabaseURL in zkproof-worker * chore(coprocessor): use DatabaseURL in txn-sender * chore(coprocessor): append default application-name to the db_url * chore(coprocessor): use DatabaseURL across all services * chore(coprocessor): add mask_database_url unit test * chore(coprocessor): append always the default app_name
goshawk-3
added a commit
that referenced
this pull request
Nov 7, 2025
* chore(coprocessor): implement a DatabaseURL wrapper * chore(coprocessor): use DatabaseURL in sns-worker * chore(coprocessor): use DatabaseURL in zkproof-worker * chore(coprocessor): use DatabaseURL in txn-sender * chore(coprocessor): append default application-name to the db_url * chore(coprocessor): use DatabaseURL across all services * chore(coprocessor): add mask_database_url unit test * chore(coprocessor): append always the default app_name
goshawk-3
added a commit
that referenced
this pull request
Nov 10, 2025
* chore(coprocessor): implement a DatabaseURL wrapper * chore(coprocessor): use DatabaseURL in sns-worker * chore(coprocessor): use DatabaseURL in zkproof-worker * chore(coprocessor): use DatabaseURL in txn-sender * chore(coprocessor): append default application-name to the db_url * chore(coprocessor): use DatabaseURL across all services * chore(coprocessor): add mask_database_url unit test * chore(coprocessor): append always the default app_name
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes https://github.com/zama-ai/fhevm-internal/issues/453
Introduces a simple wrapper around database_url string that
--database_urlis not present