fix(validator): avoid crash when policy rejection topic is disabled - #1694
Open
rohenaz wants to merge 1 commit into
Open
fix(validator): avoid crash when policy rejection topic is disabled#1694rohenaz wants to merge 1 commit into
rohenaz wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The fix correctly prevents the typed-nil interface panic for an optional producer and is backed by a targeted e2e regression test that asserts the intended behavior.
Pull request overview
This PR fixes a validator crash caused by passing a typed-nil *KafkaAsyncProducer into the kafka.KafkaAsyncProducerI interface when the optional TxPolicyRejectedConfig topic is disabled, and adds an end-to-end regression test to ensure low-fee transactions are rejected without panicking and without being persisted/assembled.
Changes:
- Ensure the policy-rejected Kafka producer remains a true
nilinterface when the topic is not configured, preventing false-positive nil checks and panics inTryPublish. - Add an e2e daemon regression test that disables
TxPolicyRejectedConfig, submits a low-fee tx, and asserts the expected policy error and no side effects (no UTXO insert, no block-assembly enqueue).
File summaries
| File | Description |
|---|---|
daemon/daemon_services.go |
Normalizes the optional policy-rejected producer to a true nil interface before wiring it into validator.NewServer. |
test/e2e/daemon/ready/optional_policy_producer_test.go |
Adds a real-daemon regression covering the “topic disabled + low-fee tx” panic path and verifies the tx is not stored or assembled. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
When
TxPolicyRejectedConfigis unset, the validator service passes a typed nil producer into aKafkaAsyncProducerIinterface. A low-fee transaction then reachespublishPolicyRejectedTx, passes its interface nil check, and panics inTryPublishinstead of returning the policy rejection.Keep the interface nil when the optional producer is absent, matching the existing local-validator construction path. Configured producers and fee policy are unchanged.
The regression starts the real local regtest daemon with SQLite, in-memory messaging and the optional topic explicitly disabled. It submits a signed transaction paying 1 sat, requires the low-fee error, and verifies that the transaction enters neither UTXO storage nor block assembly.
Validation on upstream
40faeb0f0bfcf94baf1eaa69517b5e5889af0d89:-race.git diff --checkpassed.Reproduce the focused test without external services:
The full node suite and production-scale testing were not run. This change adds no CPFP acceptance or fee-policy exception.