Repository navigation
fix(stellar): make the names TTL keeper do real work or fail closed - #254
annacodervibe wants to merge 2 commits into
Conversation
|
@annacodervibe Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
truthixify
left a comment
There was a problem hiding this comment.
Blocking: the keeper computes latestLedger + extendTo and passes that absolute ledger to extend_name_ttl. Soroban Storage.extend_ttl defines extend_to as a relative TTL in ledgers, while the contract forwards this value unchanged, so the current ledger sequence is effectively added twice and storage rent can be extended far beyond the requested 30 days. Align the contract entrypoint and keeper on relative TTL semantics and add an assertion for the resulting liveUntil delta.
|
Good catch, thanks. extend_name_ttl checked extend_to_ledger as an absolute ledger but passed it straight to extend_ttl, which takes a TTL relative to the current ledger, so the keeper’s latest + extendTo counted the current ledger twice. I changed both extend_name_ttl and bulk_renew to take extend_to as ledgers from now (0 is rejected), and the keeper now passes extend-to unchanged. I added tests that assert the resulting live_until is exactly current ledger + extend_to for both functions and for the keeper, and they fail if the old behaviour comes back. I updated the ABI snapshot by hand because I couldn’t run abi/update.sh here, so if that check complains, regenerating it should fix it. Still not run against a live testnet deployment.
|
truthixify
left a comment
There was a problem hiding this comment.
The relative-TTL fix addresses the prior semantic blocker, but this head still cannot merge. The required Stellar job reports generated bindings are out of date after the ABI change (
wraith-contracts-root@0.1.0 bindings:stellar /Users/truthixify/dev/work/wraith-protocol/contracts
tsx stellar/scripts/generate-bindings.ts
🚀 Starting Stellar TypeScript bindings generation...
⚙️ No contract IDs detected for some or all contracts. Using local WASM mode.
🔨 Compiling Soroban contracts to WASM locally (cargo build)...
✅ Local compilation succeeded!
📦 Generating bindings for "stealth-announcer" from local WASM file: target/wasm32-unknown-unknown/release/stealth_announcer.wasm
✅ Generated bindings for stealth-announcer.
📦 Generating bindings for "stealth-registry" from local WASM file: target/wasm32-unknown-unknown/release/stealth_registry.wasm
✅ Generated bindings for stealth-registry.
📦 Generating bindings for "stealth-sender" from local WASM file: target/wasm32-unknown-unknown/release/stealth_sender.wasm
✅ Generated bindings for stealth-sender.
📦 Generating bindings for "stealth-batch-sender" from local WASM file: target/wasm32-unknown-unknown/release/stealth_batch_sender.wasm
✅ Generated bindings for stealth-batch-sender.
📦 Generating bindings for "wraith-names" from local WASM file: target/wasm32-unknown-unknown/release/wraith_names.wasm
✅ Generated bindings for wraith-names.
📝 Generated top-level index.ts: bindings/typescript/index.ts
🎉 All bindings generated and post-processed successfully! produces a diff), and the new keeper CLI imports without declaring it in the workspace package dependencies. A clean install makes the three CLI-exit tests fail with . Please add the direct dependency, regenerate and commit the Stellar bindings, and rerun the keeper plus required CI/benchmark gates.
truthixify
left a comment
There was a problem hiding this comment.
The relative-TTL fix addresses the prior semantic blocker, but this head still cannot merge. The required Stellar job reports generated bindings are out of date after the ABI change, and the new keeper CLI imports commander without declaring it in the workspace dependencies. A clean install makes the three CLI-exit tests fail with ERR_MODULE_NOT_FOUND for commander. Please add the direct dependency, regenerate and commit the Stellar bindings, and rerun the keeper plus required CI and benchmark gates.
Dismissed because the review body was corrupted while posting; a corrected review follows.
Closes #228
The keeper script didn’t actually do anything. It didn’t compile, it returned an empty name list, it treated every name as at risk, and it only printed messages instead of sending transactions.
I rewrote it so it now:
builds the list of names from the contract’s register, update and release events, and can also merge in a JSON file of names if you have an index
reads each name’s real remaining TTL from the RPC
sends real extend_name_ttl transactions for names below the threshold
exits with an error if it can’t list names, can’t read a TTL, or a transaction fails
I replaced the old placeholder tests with ones that use a fake RPC, covering the dry run and each failure case, plus checks that the CLI exits nonzero.
One thing to know: I haven’t run it against a live testnet contract, so a dry run against a real deployment would be worth doing. Also, public RPC nodes only keep a few days of events, so for older names it needs either --index-file or a longer-retention RPC. It stops with an error instead of quietly skipping them.