-
Notifications
You must be signed in to change notification settings - Fork 3
🚧 fees v2 #146
🚧 fees v2 #146
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,9 +1,16 @@ | ||||||
| type: fee | ||||||
| version: 1.0.0 | ||||||
| rpc_url: https://ethereum.publicnode.com/ | ||||||
| collector_whitelist_addresses: | ||||||
| - 0x7d760c17d798a7A9a4c4AcAf311A02dC95972503 | ||||||
| - 0x5BB06B9C5e4f7624Df7100Badb2F5AA1C86d8498 | ||||||
| collector_address: 0x7d760c17d798a7A9a4c4AcAf311A02dC95972503 | ||||||
| usdc_address: 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48 | ||||||
| verifier_token: localhost-fee-apikey | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Don’t commit verifier tokens; parameterize via environment Even placeholder API keys can leak into logs/config dumps. Prefer env or secret manager injection. Apply: -verifier_token: localhost-fee-apikey
+verifier_token: ${FEES_VERIFIER_TOKEN}If you don’t perform env expansion in YAML, read this from env in code instead. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| eth_provider: ${ETH_PROVIDER_URL} | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainEnsure env var expansion is actually performed for eth_provider Many YAML loaders don’t expand ${...} by default. If the config loader doesn’t explicitly expand env vars, this will be treated as the literal string "${ETH_PROVIDER_URL}" at runtime. Run this to verify the codebase performs env expansion or reads eth_provider from env: If env expansion isn’t present, either:
🏁 Script executed: #!/bin/bash
# Look for explicit env expansion or reading of ETH_PROVIDER_URL / eth_provider
rg -n 'os\.ExpandEnv|os\.Getenv|ExpandString|envsubst|ETH_PROVIDER_URL|eth_provider' -A2
# Inspect fees config parsing for struct tags and env handling
rg -n 'EthProvider|ChainId|cronexpr|max_concurrent_jobs|success_confirmations' -A4Length of output: 8970 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---- viper occurrences ----"
rg -n "viper\." -S || true
echo
echo "---- config-related functions (ReadInConfig, Unmarshal, AutomaticEnv, SetEnvPrefix, SetEnvKeyReplacer) ----"
rg -n "ReadInConfig|Unmarshal|AutomaticEnv|SetEnvPrefix|SetEnvKeyReplacer|mapstructure" -S || true
echo
echo "---- plugin/fees/config.go ----"
sed -n '1,240p' plugin/fees/config.go || true
echo
echo "---- cmd/fees/config/config.go ----"
sed -n '1,240p' cmd/fees/config/config.go || trueLength of output: 24792 Action required: make ETH provider expansion explicit (fix required) Viper does not expand ${...} inside YAML. WithFileConfig in plugin/fees/config.go uses viper.AutomaticEnv() with prefix FEES, so the runtime env var that will override the key is FEES_ETH_PROVIDER — but the literal string "${ETH_PROVIDER_URL}" in etc/vultisig/fee.yml will not be expanded unless you preprocess the file or change the loader. Files to address
Recommended fixes (pick one)
Either document that deployments must run envsubst on fee.yml before starting, or implement the in-code expansion above. Leaving the current ${ETH_PROVIDER_URL} in the file without one of these changes will result in the literal string being used and NewFeeConfig failing the "eth_provider is required" check unless FEES_ETH_PROVIDER is set. 🤖 Prompt for AI Agents |
||||||
| chain_id: 1 | ||||||
| jobs: | ||||||
| load: | ||||||
| cronexpr: "@every 2m" | ||||||
| max_concurrent_jobs: 10 | ||||||
| transact: | ||||||
| cronexpr: "0 12 * * 5" | ||||||
| max_concurrent_jobs: 10 | ||||||
| post: | ||||||
| cronexpr: "@every 5m" | ||||||
| max_concurrent_jobs: 10 | ||||||
|
Comment on lines
+7
to
+16
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainConfirm scheduler expects these keys and cron syntax Validate that the config parser and scheduler expect:
Also confirm concurrency here configures enqueuer concurrency (not worker pool), to avoid over-enqueueing or duplicate runs. Run: Operational advice:
🏁 Script executed: #!/bin/bash
# Verify struct tags/fields for Jobs in fees config
rg -n 'type .*Job|struct \{|\byaml:"cronexpr"|\byaml:"max_concurrent_jobs"' -A3
# Verify mapping from config.jobs.* to scheduler setup
rg -n 'startLoadingFees|startTransactingFees|startPostTx|cronexpr|max_concurrent_jobs' -A4
# Verify cron parser supports "@every"
rg -n '@every|cron\.New|WithSeconds|ParseStandard'Length of output: 22380 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== go.mod ==="
sed -n '1,200p' go.mod || true
echo
echo "=== grep for robfig/cron in repo ==="
rg --hidden --no-ignore -n "robfig/cron" || true
echo
echo "=== cmd/fees/worker/main.go (full) ==="
sed -n '1,400p' cmd/fees/worker/main.go || true
echo
echo "=== plugin/fees/config.go (full) ==="
sed -n '1,260p' plugin/fees/config.go || true
echo
echo "=== search for MaxConcurrentJobs usage ==="
rg -n "MaxConcurrentJobs|max_concurrent_jobs" || true
echo
echo "=== search for asynq client/server usage ==="
rg -n "asynq.New|asynq.Client|asynq.Server|asynq.NewClient|asynq.NewServer" || true
echoLength of output: 26213 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== semaphore usages ==="
rg -n "semaphore.NewWeighted|semaphore" || true
echo
echo "=== plugin/fees/fees.go (full) ==="
sed -n '1,400p' plugin/fees/fees.go || true
echo
echo "=== plugin/fees/post_tx.go (full) ==="
sed -n '1,400p' plugin/fees/post_tx.go || true
echoLength of output: 14752 🌐 Web query: 💡 Result: Short answer: Yes.
If you need a seconds field (6-field/quartz-like) or want to customise parsing, construct the Cron with options, e.g.:
If you want, I can show a tiny code example demonstrating AddFunc with "@every 1h" and with "0 12 * * 5". Citations:
Confirm cron keys & syntax; concurrency is worker-side (semaphores), not the enqueuer Snippet (unchanged): Summary (very short):
Files to review / attention:
Actionable recommendation (short):
🧰 Tools🪛 YAMLlint (1.37.1)[error] 16-16: no new line character at the end of file (new-line-at-end-of-file) 🤖 Prompt for AI Agents |
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,162 @@ | ||||||||||||||||||||||||||||||||||
| # Fees plugin | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| The fees pluigin is responsible for collecting user based fees incurred from other plugins | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+3
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Adopt singular "fee" naming and fix typo in intro Aligns with the project's naming convention and fixes a spelling error. Apply: -# Fees plugin
+# Fee plugin
@@
-The fees pluigin is responsible for collecting user based fees incurred from other plugins
+The fee plugin is responsible for collecting user-based fees incurred from other pluginsAdditionally, consider renaming this file to FEE.md to fully comply with the “use fee (singular)” guideline. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ## Overview | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| As this is a 2 part system, and the fees plugin works differently from other plugins. Here is an overview of the current architecture. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ### Verifier | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| The verifier is responsible for tracking the fees which need to be collected. This is achieved by creating a `billing` record against each plugin policy. The `billing` record contains information which is encoded in the `FeePolicy` struct of the recipe that a user signs. There may be zero, one, or several fee policies, and hence `billing` records connected to a `plugin policy`. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| When a new plugin policy is created this data is extracted, verified (against the `pricings` (definitions) table) and then inserted accordingly in the db. It is also synced to the plugin server and if it fails everything fails. The following types of `billing` record exist. They are all recorded as enums in the db and in the implementation | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| - `once` - **fully implemented** one billing record and one `fee` record are created on policy insert. | ||||||||||||||||||||||||||||||||||
| - `recurring` - **mostly implemented** the db views and code is there but there is no scheduled job to create the fee records at intervals. | ||||||||||||||||||||||||||||||||||
| - `tx` - **barely implemented** the enum types exist but the code to incur fees upon certain tx based transactions is not yet there. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### Fee records | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Each fee has a unique ID in the system and **must** be connected to a `billing` record. Several fees may share the same `billing` record in the cases of recurrent or tx based billing records. In the case of one time fees, the fee is incurred instantly on successful install, otherwise the fee creations will be handled via trigger. Fees are known to be collected successfully by the use of a `collected_at` attribute and the `charged_at` as being set as the date the fee was incurred which is pre-empted to be needed for audit purposes. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### Tx signing | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| - *currently pending pr* - fee ids are passed along with a signing request which are used to verify that the amount being requested for a signature is the same. | ||||||||||||||||||||||||||||||||||
| - *current* - the system checks for all fees which are due to be collected and if the amount doesn't match it rejects a sign request. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
Comment on lines
+25
to
+27
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Update Tx signing notes: remove “pending pr” and reflect current behavior The PR introduces MarkFeeAsCollected and fee-id-aware signing. Remove “pending pr” and document the actual behavior. -#### Tx signing
-
-- *currently pending pr* - fee ids are passed along with a signing request which are used to verify that the amount being requested for a signature is the same.
-- *current* - the system checks for all fees which are due to be collected and if the amount doesn't match it rejects a sign request.
+#### Tx signing
+
+- Fee IDs are included with the signing request and validated so the requested amount matches the sum of pending fees.
+- If the amount does not match, the sign request is rejected.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| The signing request from the plugin server to the verifier is parsed and put through the above checks if the `policy_id` is that of a fee plugin. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ### Plugin | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| In the `v2` implementation there are 3 distinct processes for handling fees. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| `loading` - pulls in the fees from the verifier and assigns them to a `fee run` (more later) | ||||||||||||||||||||||||||||||||||
| `transaction` - iterates through each of the fee runs, builds transactions, sends them to the verifier to be signed and then broadcasts them | ||||||||||||||||||||||||||||||||||
| `post` - checks the sent transactions and if successful updates the verifier which marks a value as `collected_at` **pending pr** (at the same time a `treasury_ledger` table is appended to with the fees which go to a developer and vultisig.) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
Comment on lines
+32
to
+37
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Name the stages consistently and remove “pending pr” in post step Align with function names and current verifier POST API. -In the `v2` implementation there are 3 distinct processes for handling fees.
+In the `v2` implementation there are 3 distinct processes for handling fee collection.
@@
-`loading` - pulls in the fees from the verifier and assigns them to a `fee run` (more later)
-`transaction` - iterates through each of the fee runs, builds transactions, sends them to the verifier to be signed and then broadcasts them
-`post` - checks the sent transactions and if successful updates the verifier which marks a value as `collected_at` **pending pr** (at the same time a `treasury_ledger` table is appended to with the fees which go to a developer and vultisig.)
+`loading` (LoadFees) - pulls fees from the verifier and assigns them to a fee run
+`transaction` (HandleTransactions) - iterates through fee runs, builds transactions, requests TSS signing, and broadcasts them
+`post` (HandlePostTx) - checks sent transactions and, on sufficient confirmations, calls VerifierApi.MarkFeeAsCollected to set `collected_at`📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| #### Fee Runs | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| A `fee run` is a logical grouping of fees. They have various states depending on their lifecycle. When loading fees if a fee is found which has not yet been transacted *and* there is a `pending` (aka draft) fee run then the fee is included with it. If no fee run is detected then a new fee run is created. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| The db structure are different between verifier and plugin here due to a separation of concerns. Verifier needs to create fee entries, track their state across their lifecycle and handle treasury output from them. The plugin server simply needs to group them and track their ids and amounts. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ### Detailed Workflow | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### 1. Fee Loading Process (`LoadFees`) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| **Trigger**: Scheduled cron job (configurable interval, default: every 10 minutes) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
Comment on lines
+48
to
+49
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainAlign schedule with current config defaults Doc says “every 10 minutes” but fee.yml uses @every 2m per the PR summary. Adjust or explicitly reference config. -**Trigger**: Scheduled cron job (configurable interval, default: every 10 minutes)
+**Trigger**: Scheduled cron job (configurable; default: @every 2m, see fee.yml)If the default has changed, please update fee.yml accordingly or confirm the intended default. 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Repo root: $(pwd) ==="
echo
echo "1) Find files with 'fee' in the name (yml/yaml):"
find . -type f \( -iname "*fee*.yml" -o -iname "*fee*.yaml" \) -print || true
echo
echo "2) Ripgrep for '@every' and 'fee.yml' occurrences:"
rg --hidden -n --no-ignore-vcs -S '@every' || true
rg --hidden -n --no-ignore-vcs -S 'fee\.ya?ml' || true
echo
echo "3) Show content of likely fee config files (first 200 lines each):"
files=$(find . -type f \( -iname "*fee*.yml" -o -iname "*fee*.yaml" \) -print)
if [ -z "$files" ]; then
echo "No fee*.yml/yaml files found by name. Searching for 'fee' directories under plugin/fees..."
files=$(find plugin -type f -iname "*.yml" -o -iname "*.yaml" | rg -n 'fees|fee' || true)
fi
if [ -n "$files" ]; then
echo "$files" | while read -r f; do
echo
echo "---- $f ----"
sed -n '1,200p' "$f" || true
done
else
echo "No fee-related yaml files found to display."
fiLength of output: 2145 Align FEES.md schedule with actual config defaults FEES.md states "every 10 minutes" but the repo config uses different defaults (load = "@every 2m", post = "@every 5m"). Update the docs or confirm/change the config. Files to update / check:
Suggested change: -**Trigger**: Scheduled cron job (configurable interval, default: every 10 minutes)
+**Trigger**: Scheduled cron job (configurable; defaults: load @every 2m, post @every 5m — see etc/vultisig/fee.yml)If the intended default is different, update etc/vultisig/fee.yml and plugin/fees/config.go to match, or confirm and keep the doc change. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| **Process**: | ||||||||||||||||||||||||||||||||||
| 1. Retrieves all fee policies from database | ||||||||||||||||||||||||||||||||||
| 2. For each policy, queries verifier API for pending fees | ||||||||||||||||||||||||||||||||||
| 3. Validates fee amounts and consistency | ||||||||||||||||||||||||||||||||||
| 4. Creates or updates fee runs in `draft` status | ||||||||||||||||||||||||||||||||||
| 5. Adds individual fees to fee runs | ||||||||||||||||||||||||||||||||||
| 6. Uses semaphore for concurrent processing (configurable limit). | ||||||||||||||||||||||||||||||||||
| 7. Will only run when one of the other 3 processes isn't running | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| **Key Features**: | ||||||||||||||||||||||||||||||||||
| - Concurrent processing with semaphore limiting | ||||||||||||||||||||||||||||||||||
| - Duplicate fee detection and prevention | ||||||||||||||||||||||||||||||||||
| - Automatic fee run creation and management | ||||||||||||||||||||||||||||||||||
| - Error handling and rollback capabilities | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### 2. Transaction Handling (`HandleTransactions`) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| **Trigger**: Scheduled cron job (configurable interval, default: fridays weekly) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| **Process**: | ||||||||||||||||||||||||||||||||||
| 1. Retrieves all fee runs in `draft` status | ||||||||||||||||||||||||||||||||||
| 2. For each valid fee run: | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+65
to
+71
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Document run-based transaction proposal and the intentionally unimplemented public method Add an explicit note to prevent confusion for integrators. -#### 2. Transaction Handling (`HandleTransactions`)
+#### 2. Transaction Handling (`HandleTransactions`)
+
+Note: The public `ProposeTransactions` method intentionally returns “not implemented” for the fee plugin v2. The actual logic uses a run-based flow implemented in the private `proposeTransactions(types.FeeRun)` method.
@@
-**Trigger**: Scheduled cron job (configurable interval, default: fridays weekly)
+**Trigger**: Scheduled cron job (configurable; default: 0 12 * 5, i.e. Fridays at 12:00 UTC)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| - Generates Ethereum ERC20 USDC transfer transaction | ||||||||||||||||||||||||||||||||||
| - Creates keysign request with transaction data | ||||||||||||||||||||||||||||||||||
| - Initiates signing process through TSS | ||||||||||||||||||||||||||||||||||
| - Broadcasts signed transaction to blockchain | ||||||||||||||||||||||||||||||||||
| - Updates fee run status to `sent` | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| **Transaction Details**: | ||||||||||||||||||||||||||||||||||
| - **Token**: USDC (ERC20) | ||||||||||||||||||||||||||||||||||
| - **Chain**: Ethereum (chainId: 1) | ||||||||||||||||||||||||||||||||||
| - **Recipient**: Vultisig Treasury (resolved via magic constants) | ||||||||||||||||||||||||||||||||||
| - **Gas Limit**: 65,000 (typical ERC20 transfer upper bound) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### 3. Post-Transaction Processing (`HandlePostTx`) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| **Trigger**: Scheduled cron job (configurable interval, default: every 10 minutes) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| **Process**: | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+86
to
+88
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainAlign post-tx schedule with current config defaults Matches the fee.yml example in the PR summary. -**Trigger**: Scheduled cron job (configurable interval, default: every 10 minutes)
+**Trigger**: Scheduled cron job (configurable; default: @every 5m, see fee.yml)Please confirm the intended default and update either the doc or config if they differ. 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---- plugin/fees/FEES.md (lines 1-200) ----"
if [ -f plugin/fees/FEES.md ]; then
sed -n '1,200p' plugin/fees/FEES.md
else
echo "plugin/fees/FEES.md not found"
fi
echo
echo "---- Searching repository for '@every', 'every 5m', 'every 10m', or '10 minutes' ----"
rg -n --hidden -S '@every|every\s*5m|every\s*10m|10\s*minutes' || true
echo
echo "---- Searching repository for fee.yml/fee.yaml/fees.yml/fees.yaml strings ----"
rg -n --hidden -S 'fee\.yml|fee\.yaml|fees\.yml|fees\.yaml' || true
echo
echo "---- Show plugin/fees/fee.yml if present ----"
if [ -f plugin/fees/fee.yml ]; then
sed -n '1,200p' plugin/fees/fee.yml
else
echo "plugin/fees/fee.yml not found"
fi
echo
echo "---- Show root fee.yml if present ----"
if [ -f fee.yml ]; then
sed -n '1,200p' fee.yml
else
echo "fee.yml not found at repo root"
fiLength of output: 9886 Align scheduling defaults in FEES.md with config values FEES.md documents "default: every 10 minutes" for the LoadFees and HandlePostTx jobs, but the repository defaults are @every 2m (load) and @every 5m (post). Update the docs or the config so they match. Files/locations to fix:
Suggested diffs for FEES.md: -**Trigger**: Scheduled cron job (configurable interval, default: every 10 minutes)
+**Trigger**: Scheduled cron job (configurable; default: @every 2m — see plugin/fees/config.go or etc/vultisig/fee.yml)(for LoadFees) -**Trigger**: Scheduled cron job (configurable interval, default: every 10 minutes)
+**Trigger**: Scheduled cron job (configurable; default: @every 5m — see plugin/fees/config.go or etc/vultisig/fee.yml)(for HandlePostTx) Please update the docs or adjust the config if the intended defaults are different. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||
| 1. Monitors all fee runs in `sent` status | ||||||||||||||||||||||||||||||||||
| 2. Checks transaction receipts on blockchain | ||||||||||||||||||||||||||||||||||
| 3. Validates confirmation count against configured threshold | ||||||||||||||||||||||||||||||||||
| 4. Updates verifier with collection status | ||||||||||||||||||||||||||||||||||
| 5. Marks fee runs as `completed` or `failed` | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| **Confirmation Logic**: | ||||||||||||||||||||||||||||||||||
| - Waits for configurable number of confirmations | ||||||||||||||||||||||||||||||||||
| - Handles transaction failures and rebroadcast scenarios | ||||||||||||||||||||||||||||||||||
| - Maintains state consistency between plugin and verifier | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ### Job Scheduling & Configuration | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### Asynq Task Types | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| 1. **`fees:load`** - Fee loading from verifier | ||||||||||||||||||||||||||||||||||
| 2. **`fees:transaction`** - Transaction creation and broadcasting | ||||||||||||||||||||||||||||||||||
| 3. **`fees:post_tx`** - Post-transaction status checking | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### Policy Validation | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| The system validates fee policies against strict criteria: | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| 1. **Resource Validation**: Only `ethereum.erc20.transfer` operations allowed | ||||||||||||||||||||||||||||||||||
| 2. **Recipient Validation**: Must use `VULTISIG_TREASURY` magic constant | ||||||||||||||||||||||||||||||||||
| 3. **Amount Constraints**: Maximum fee amount enforcement | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+112
to
+114
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainRestrict resource to USDC transfer per policy constraints Project policy is USDC-only, not general ERC20. -1. **Resource Validation**: Only `ethereum.erc20.transfer` operations allowed
+1. **Resource Validation**: Only `ethereum.usdc.transfer` operations allowedTo verify consistency across the repo: 🏁 Script executed: #!/bin/bash
# Check occurrences of resource names and verify USDC-only constraint mentioned in policy/validation code and docs.
rg -n "ethereum\.erc20\.transfer|ethereum\.usdc\.transfer" -A 2Length of output: 1686 Restrict fees plugin to USDC transfers — change required FEES.md and the fees plugin contain inconsistent resource names; the fees plugin must be USDC-only. Files needing changes:
Note: plugin/payroll/transaction.go contains Updated diff (docs): -1. **Resource Validation**: Only `ethereum.erc20.transfer` operations allowed
+1. **Resource Validation**: Only `ethereum.usdc.transfer` operations allowed(Used retrieved learning confirming the fees plugin is USDC-only.) 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| 4. **Recipe Schema**: Validates against predefined recipe specification | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### Transaction Security | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| 1. **Mutex Protection**: Prevents concurrent transaction operations | ||||||||||||||||||||||||||||||||||
| 2. **Amount Verification**: Cross-validates amounts between verifier and plugin | ||||||||||||||||||||||||||||||||||
| 3. **Signature Validation**: Uses TSS for secure transaction signing | ||||||||||||||||||||||||||||||||||
| 4. **Rollback Mechanisms**: Database transactions ensure consistency | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ### API Integration | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### Verifier API Endpoints | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| 1. **`GetPublicKeysFees(publicKey)`** - Retrieves pending fees for a vault | ||||||||||||||||||||||||||||||||||
| 2. **`MarkFeeAsCollected(txHash, timestamp, feeIds...)`** - Marks fees as collected | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
Comment on lines
+128
to
+130
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainSpecify HTTP endpoints and payload for verifier integration Make the API contract explicit and remove ambiguity around method names. -1. **`GetPublicKeysFees(publicKey)`** - Retrieves pending fees for a vault
-2. **`MarkFeeAsCollected(txHash, timestamp, feeIds...)`** - Marks fees as collected
+1. GET `/fees?publicKey={publicKey}` - Retrieves pending fees for a vault
+2. POST `/fees/collected` - Marks fees as collected
+ Body (JSON):
+ {
+ "txHash": "0x...",
+ "collectedAt": "2025-08-01T12:34:56Z",
+ "feeIds": ["fee_123", "fee_456"]
+ }If the actual endpoints or parameter names differ, please update the doc to match the implementation. You can confirm with: 🏁 Script executed: #!/bin/bash
# Discover the verifier client methods and routes
rg -n "MarkFeeAsCollected|/fees/collected|GetPublicKey|GetPublicKeysFees|/fees\?" -A 3Length of output: 1908 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,240p' internal/verifierapi/fees.goLength of output: 3044 Update FEES.md to match verifier API (endpoints & JSON payloads) The verifier implements GET /fees/publickey/{ecdsaPublicKey} and POST /fees/collected with snake_case JSON keys — update the doc to match. Files to update / reference:
Suggested diff: -1. **`GetPublicKeysFees(publicKey)`** - Retrieves pending fees for a vault
-2. **`MarkFeeAsCollected(txHash, timestamp, feeIds...)`** - Marks fees as collected
+1. GET `/fees/publickey/{ecdsaPublicKey}` - Retrieves pending fees for a vault
+ Response: 200 JSON -> APIResponse with "data" containing FeeHistoryDto
+2. POST `/fees/collected` - Marks fees as collected
+ Body (JSON):
+ {
+ "ids": ["fee-uuid-1", "fee-uuid-2"],
+ "tx_hash": "0x...",
+ "collected_at": "2025-08-01T12:34:56Z" // ISO8601 timestamp
+ }
+ Response: 200 OK📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| ### Monitoring & Observability | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### Logging Structure | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| All operations use structured logging with consistent field names: | ||||||||||||||||||||||||||||||||||
| - `publicKey`: Vault public key | ||||||||||||||||||||||||||||||||||
| - `feeId`/`feeIds`: Individual fee identifiers | ||||||||||||||||||||||||||||||||||
| - `runId`: Fee run identifier | ||||||||||||||||||||||||||||||||||
| - `policyId`: Plugin policy identifier | ||||||||||||||||||||||||||||||||||
| - `tx_hash`: Blockchain transaction hash | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ### Implementation Status | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### Fully Implemented Features | ||||||||||||||||||||||||||||||||||
| - `once` billing type - Single fee collection on policy creation | ||||||||||||||||||||||||||||||||||
| - Fee loading and aggregation system | ||||||||||||||||||||||||||||||||||
| - ERC20 USDC transaction generation | ||||||||||||||||||||||||||||||||||
| - TSS-based transaction signing | ||||||||||||||||||||||||||||||||||
| - Post-transaction confirmation tracking | ||||||||||||||||||||||||||||||||||
| - Database schema and migrations | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### Partially Implemented Features | ||||||||||||||||||||||||||||||||||
| - `recurring` billing type - Database structure exists, scheduling incomplete | ||||||||||||||||||||||||||||||||||
| - Transaction rebroadcast logic - Framework present, full implementation pending | ||||||||||||||||||||||||||||||||||
| - Failed transaction recovery - Basic structure, comprehensive handling needed | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #### Future Enhancements | ||||||||||||||||||||||||||||||||||
| - `tx` billing type - Transaction-based fee collection | ||||||||||||||||||||||||||||||||||
| - Multi-token support beyond USDC | ||||||||||||||||||||||||||||||||||
| - Cross-chain fee collection capabilities | ||||||||||||||||||||||||||||||||||
| - Advanced retry and recovery mechanisms | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably have a mutex - it's possible that in theory the cron run could trigger while the previous run hasn't completed yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a mutex in the service itself which stops loads and txs happening synchronously