Skip to content

fix: critical SQL syntax error in BFT _apply_settlement blocks all reward payouts#1825

Merged
Scottcjn merged 1 commit into
Scottcjn:mainfrom
AliaksandrNazaruk:fix/bft-sql-comment-bug
Mar 24, 2026
Merged

fix: critical SQL syntax error in BFT _apply_settlement blocks all reward payouts#1825
Scottcjn merged 1 commit into
Scottcjn:mainfrom
AliaksandrNazaruk:fix/bft-sql-comment-bug

Conversation

@AliaksandrNazaruk
Copy link
Copy Markdown
Contributor

Bug

In node/rustchain_bft_consensus.py, the _apply_settlement() method has Python-style # comments inside a SQL string literal (lines 614-615):

conn.execute("""
    INSERT INTO balances (miner_id, amount_i64)
    VALUES (?, ?)
    ON CONFLICT(miner_id) DO UPDATE SET
    amount_i64 = amount_i64 + excluded.amount_i64
# Store as integer micro-RTC ...
# floating-point drift ...
""", (miner_id, int(reward * 1_000_000)))

SQLite does not support # as a comment delimiter (only -- and /* */). This causes:

sqlite3.OperationalError: unrecognized token: "#"

Impact

CRITICAL — Every time BFT consensus reaches quorum and calls _apply_settlement(), the method crashes. No miner rewards are ever distributed through the BFT engine.

Verification

import sqlite3
conn = sqlite3.connect(':memory:')
conn.execute('CREATE TABLE t (a INT)')
conn.execute('INSERT INTO t VALUES (1) # comment')
# → OperationalError: unrecognized token: "#"

Fix

Move the comments outside the SQL string literal. The explanation is preserved as Python comments above the conn.execute() call.

Bounty: #305 (Bug Report, 5-15 RTC)

The # comments on lines 614-615 were inside a SQL string literal passed to
sqlite3.execute(). SQLite does not support # as a comment delimiter (only --
and /* */), causing an 'unrecognized token' error at runtime.

This means _apply_settlement() would crash every time BFT consensus reached
quorum, silently preventing ALL miner reward payouts via the BFT engine.

Fix: move the comments outside the SQL string while preserving the explanation.

Severity: CRITICAL — blocks all BFT reward distribution.
@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/XS PR: 1-10 lines labels Mar 24, 2026
@Scottcjn Scottcjn merged commit d3a4763 into Scottcjn:main Mar 24, 2026
3 checks passed
@AliaksandrNazaruk
Copy link
Copy Markdown
Contributor Author

Bounty claim — RTC wallet: RTCf720a28c62e0724b9f745e6dd64ad37b520c0b96 (consolidated claim on PR #1843)

Copy link
Copy Markdown
Contributor

@FlintLeng FlintLeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this PR — clean implementation with good attention to edge cases.

Copy link
Copy Markdown
Contributor

@FlintLeng FlintLeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #1825 — Review:

Code formatting/comment cleanup on micro-RTC integer storage. The logic is correct — storing as integer micro-RTC avoids floating-point drift. Cosmetic change maintaining correct documentation. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/XS PR: 1-10 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants