Repository navigation
Fix bug balances, updatable metadata event, publish/retrieve last commit balances - #4
philogicae wants to merge 6 commits into
Conversation
Co-authored-by: Bram <cortex@worlddomination.be>
There was a problem hiding this comment.
The PR introduces a well-structured voucher balance system with a singleton for state management and aleph-backed checkpointing, but has two genuine bugs: a KeyError crash in recompute_all_balances when vouchers expire, and check_metadata/check_balance will crash on None metadata without retry. Several medium issues also need attention: bare except: clauses, blocking requests.get without a timeout on the async event loop, and a misleading log message in the last-commit fetch path.
src/aleph_nodestatus/voucher.py (line 150): Bug – KeyError when vouchers expire. recompute_all_balances iterates voucher_settings.balances (old snapshot) and looks up each claimer in the freshly-computed balances dict. If a claimer's vouchers have all expired, they appear in the old dict but not the new one, causing an unhandled KeyError that propagates up and corrupts state.
Fix: use .get(claimer, 0) and include the expired case as a diff:
for claimer, old_balance in voucher_settings.balances.items():
new_balance = balances.get(claimer, 0)
if old_balance != new_balance:
diffs.append((claimer, new_balance - old_balance))src/aleph_nodestatus/voucher.py (line 117): Bug – None metadata cached and then dereferenced. fetch_metadata returns None implicitly on any exception. check_metadata then stores that None permanently (voucher_settings.metadata[metadata_id] = None), so future calls return the cached None without retrying. When check_balance (or recompute_all_balances) then does metadata["attributes"] it raises TypeError.
Two fixes needed: (1) don't cache None in check_metadata, and (2) guard against None in check_balance / recompute_all_balances.
src/aleph_nodestatus/voucher.py (line 110): Medium – blocking HTTP call with no timeout on the async event loop. requests.get(url) is synchronous and has no timeout. A slow or hung metadata endpoint will stall the entire event loop. Use a timeout (e.g. requests.get(url, timeout=10)) or switch to httpx.AsyncClient / aiohttp.
src/aleph_nodestatus/voucher.py (line 170): Medium – bare except: swallows KeyboardInterrupt, SystemExit, GeneratorExit. Change to except Exception: to avoid accidentally catching signals and interpreter-exit exceptions.
src/aleph_nodestatus/voucher.py (line 30): Medium – singleton left permanently broken if __init__ fails mid-way. self.initialized = True is set on line 30, before get_web3(), get_contract(), etc. If any of those raise, subsequent VoucherSettings() calls will see initialized and skip re-initializati
AD10
on, leaving a half-constructed singleton forever. Move self.initialized = True to the last line of the block, or wrap the entire block in try/except and re-raise without setting the flag.
src/aleph_nodestatus/voucher.py (line 172): Minor – misleading log. "commit found for last_height=0" is logged even when all posts existed but none matched the contract address filter (so last_commit is still None and chain_height is still 0). Add a guard: if last_commit: before logging "commit found", and log "no matching commit" otherwise.
src/aleph_nodestatus/erc20.py (line 137): Minor – no-op assignment. balances = balances does nothing. Remove it.
src/aleph_nodestatus/erc20.py (line 74): Minor – commented-out dead code. #and voucher_settings.active:, #voucher_settings.active = False, the two commented-out imports at the top, and the LOGGER.error / asyncio.sleep block should either be removed or tracked in an issue. Leaving large swaths of commented code in a PR makes the intent unclear and adds maintenance noise.
src/aleph_nodestatus/voucher.py (line 16): Nit – mutable class-level attributes. balances = {}, vouchers = {}, and metadata = {} are defined as class attributes. While harmless here due to the singleton, they're shadowed by instance attributes only after first assignment. Better to initialize them in __init__ as instance attributes to make the intent clear and avoid accidental class-level sharing if the singleton assumption ever changes.
src/aleph_nodestatus/voucher.py (line 194): Nit – missing newline at end of file.
Example of a vouchers-update post:
https://explorer.aleph.im/address/ETH/0x3E1aba4ad853Dd7Aa531aB59F10bd9f4d89aebaF/message/POST/b665bb960de05b44c827ad310ee1fdfe0056be0fda1072106f153f9b7baf0221