feat: add DisableIndicatorAutoCreate flag to prevent implicit balance…#278
feat: add DisableIndicatorAutoCreate flag to prevent implicit balance…#278DaveMaks wants to merge 1 commit intoblnkfinance:mainfrom
Conversation
… creation Adds config option (env: BLNK_TRANSACTION_DISABLE_INDICATOR_AUTO_CREATE) that blocks automatic balance creation when an @Indicator is not found.
There was a problem hiding this comment.
Pull request overview
Adds a transaction-level configuration flag to disable implicit balance creation when resolving @indicator references, to prevent accidental money-supply changes from typos or unintended indicators.
Changes:
- Introduce
disable_indicator_auto_create/BLNK_TRANSACTION_DISABLE_INDICATOR_AUTO_CREATEinTransactionConfig. - Enforce the flag in indicator resolution by rejecting settlement when an indicator can’t be found (instead of auto-creating a balance).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
config/config.go |
Adds DisableIndicatorAutoCreate to transaction configuration (JSON + env var). |
balance.go |
Adds strict-mode gate to getOrCreateBalanceByIndicator to block implicit creation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| balance, err := l.datasource.GetBalanceByIndicator(indicator, currency) | ||
| if err != nil { | ||
| if cfg.Transaction.DisableIndicatorAutoCreate { | ||
| span.RecordError(err) | ||
| return nil, fmt.Errorf("balance with indicator '%s' not found and auto-creation is disabled", indicator) | ||
| } |
There was a problem hiding this comment.
The strict-mode branch triggers on any error from GetBalanceByIndicator, but that method can return non-"not found" errors (e.g., DB/query failures, data parse errors). In those cases this code will incorrectly report "not found" and also drops the original error from the returned error value. Consider only treating the error as "missing balance" when it is actually a not-found condition, and otherwise return/wrap the original error so operational failures aren’t masked.
| if cfg.Transaction.DisableIndicatorAutoCreate { | ||
| span.RecordError(err) | ||
| return nil, fmt.Errorf("balance with indicator '%s' not found and auto-creation is disabled", indicator) | ||
| } |
There was a problem hiding this comment.
This new behavior (rejecting @indicator resolution when DisableIndicatorAutoCreate is enabled) isn’t covered by tests. Please add unit/integration coverage that asserts: (1) with the flag enabled, a transaction referencing a non-existent indicator fails and no balance is created; and (2) with the flag disabled, the previous auto-create behavior still works.
| if err != nil { | ||
| if cfg.Transaction.DisableIndicatorAutoCreate { | ||
| span.RecordError(err) | ||
| return nil, fmt.Errorf("balance with indicator '%s' not found and auto-creation is disabled", indicator) |
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
|
Hi @DaveMaks, Thank you for the PR and the addition. I looked through it, and I think it's a good addition for safety reasons. Comment: This might require modifying the create balance to accept an indicator and to validate that the ledger id is general_ledger_id when that's passed. Thanks |
Protecting Money Supply Consistency
When using the @Indicator syntax in transactions, the system automatically created a new balance if it didn't exist. This behavior is convenient during initial setup, but in a production environment, it poses a risk: an accidental typo in the indicator name or an intentional reference to a non-existent balance would silently create a new balance and credit funds "out of thin air"—without an appropriate source.
The disable_indicator_auto_create: true flag switches the system to strict mode: if a balance with the specified indicator isn't found, the transaction is rejected with an error. All balances must be explicitly created via the API before settlement. This ensures that the overall money supply in the system is changed only through explicit and controlled transactions.