fix(dao): correct APC math for withdrawing cells (Telegram 9)#276
Conversation
…g cells (Telegram 9) User reported the DAO APC reading 1.55% which felt low. Investigation found that for cells in the withdrawing state, the APC calculation used wall-clock `now` as the upper bound on elapsed time, but compensation stops accruing the moment a deposit moves to withdrawing. Wall-clock keeps ticking past that point, so dividing by a longer window understates the APC. Fix: for withdrawing cells, use the withdraw-block header timestamp as the endpoint. For deposited (still-accruing) cells, keep wall-clock `now`. The deposit-side timestamp is unchanged in both branches. Also added INFO-level logging of (compensation, capacity, elapsedDays, apc, isWithdrawing) so the next time a user reports an APC value we have the raw inputs to verify in logcat without instrumentation. This may not move 1.55% all the way to the user's expected number — actual CKB DAO compensation has been declining as secondary issuance progresses, so values in the 1.5-2% range are within normal — but the math is now correct for the withdrawing case which is the only branch with an identifiable error. Builds clean. Unit tests pass. Refs Telegram bug report (georgiev, 2026-05-21)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough
ChangesAPC state-dependent calculation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@android/app/src/main/java/com/rjnr/pocketnode/data/gateway/DaoDepositReader.kt`:
- Around line 259-263: For withdrawing cells, remove the wall-clock fallback
used when computing endTimeMs (the if branch using isWithdrawing and
cellBlockHeader?.timestamp) and instead detect missing or malformed timestamps:
attempt to parse cellBlockHeader.timestamp (reference symbols: endTimeMs,
isWithdrawing, cellBlockHeader.timestamp, compensation, apc) inside a safe parse
(try/catch or runCatching) and if parsing fails or timestamp is null do not
compute APC (leave apc as 0.0) and emit a warning log indicating the
missing/invalid withdraw block timestamp for that cell; only for non-withdrawing
cells continue using System.currentTimeMillis() as before. Ensure
NumberFormatException is caught so parsing errors don’t fall back to wall-clock
time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a7a24705-611b-4a76-833f-06b9de3c8a49
📒 Files selected for processing (1)
android/app/src/main/java/com/rjnr/pocketnode/data/gateway/DaoDepositReader.kt
| val endTimeMs = if (isWithdrawing) { | ||
| cellBlockHeader?.timestamp?.removePrefix("0x")?.toLong(16) ?: System.currentTimeMillis() | ||
| } else { | ||
| System.currentTimeMillis() | ||
| } |
There was a problem hiding this comment.
The wall-clock fallback for withdrawing cells defeats the purpose of the fix.
For withdrawing cells, if cellBlockHeader?.timestamp is null or cannot be parsed, falling back to System.currentTimeMillis() reintroduces the bug this PR aims to fix: the elapsed window would be overstated and APC understated.
If the withdraw block timestamp is unavailable for a withdrawing cell where compensation > 0, the data is inconsistent (compensation requires the header). Rather than silently producing incorrect APC with the wall-clock fallback, consider:
- Skipping APC calculation (
apcremains 0.0) - Logging a warning about the missing timestamp
Additionally, toLong(16) can throw NumberFormatException if the hex string is malformed; the safe-call operator only handles null, not exceptions.
🛡️ Proposed fix to handle missing timestamp safely
- val endTimeMs = if (isWithdrawing) {
- cellBlockHeader?.timestamp?.removePrefix("0x")?.toLong(16) ?: System.currentTimeMillis()
- } else {
- System.currentTimeMillis()
- }
+ val endTimeMs = if (isWithdrawing) {
+ val timestampHex = cellBlockHeader?.timestamp?.removePrefix("0x")
+ if (timestampHex != null) {
+ try {
+ timestampHex.toLong(16)
+ } catch (e: NumberFormatException) {
+ Log.w(TAG, "Malformed timestamp for withdrawing cell $cellId, skipping APC calculation", e)
+ -1L // sentinel to skip APC calculation below
+ }
+ } else {
+ Log.w(TAG, "Missing timestamp for withdrawing cell $cellId, skipping APC calculation")
+ -1L // sentinel to skip APC calculation below
+ }
+ } else {
+ System.currentTimeMillis()
+ }
val elapsedDays = (endTimeMs - depositTimestampMs) / 86_400_000.0
- if (elapsedDays >= 1.0) {
+ if (endTimeMs > 0 && elapsedDays >= 1.0) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@android/app/src/main/java/com/rjnr/pocketnode/data/gateway/DaoDepositReader.kt`
around lines 259 - 263, For withdrawing cells, remove the wall-clock fallback
used when computing endTimeMs (the if branch using isWithdrawing and
cellBlockHeader?.timestamp) and instead detect missing or malformed timestamps:
attempt to parse cellBlockHeader.timestamp (reference symbols: endTimeMs,
isWithdrawing, cellBlockHeader.timestamp, compensation, apc) inside a safe parse
(try/catch or runCatching) and if parsing fails or timestamp is null do not
compute APC (leave apc as 0.0) and emit a warning log indicating the
missing/invalid withdraw block timestamp for that cell; only for non-withdrawing
cells continue using System.currentTimeMillis() as before. Ensure
NumberFormatException is caught so parsing errors don’t fall back to wall-clock
time.
1 participant
Summary
Telegram bug 9: user (georgiev) reported "dao apc is wrong i think, it's says 1.55%". Investigation found a real bug in the math for withdrawing cells.
Root cause
DaoDepositReader.kt:246 used `(now - depositTime)` as elapsed time for the APC calc:
```kotlin
val elapsedDays = (System.currentTimeMillis() - depositTimestampMs) / 86_400_000.0
apc = (compensation / capacity) / (elapsedDays / 365.25) * 100.0
```
For a deposited cell that's still accruing, `now` is the correct endpoint.
For a withdrawing cell, compensation stops accruing the moment the deposit transitions to withdrawing. Wall-clock keeps advancing past that point, so dividing by a longer window understates the realised APC.
Fix
Branch on `isWithdrawing`. For withdrawing cells, use the withdraw-block header timestamp as the endpoint (same source the compensation amount itself comes from). For deposited cells, keep wall-clock `now`.
```kotlin
val endTimeMs = if (isWithdrawing) {
cellBlockHeader?.timestamp?.removePrefix("0x")?.toLong(16)
?: System.currentTimeMillis()
} else {
System.currentTimeMillis()
}
val elapsedDays = (endTimeMs - depositTimestampMs) / 86_400_000.0
```
Also added DEBUG-level logging of `(compensation, capacity, elapsedDays, apc, isWithdrawing)` so a future APC report can be cross-checked against logcat without needing instrumentation.
Caveat
This may not move 1.55% all the way to the user's expected number — actual CKB DAO compensation has been declining as secondary issuance progresses, and values in the 1.5-2.0% range are within normal. But the math is now correct for the withdrawing case, which is the only branch with an identifiable error.
Test plan
Refs Telegram bug report (georgiev, 2026-05-21)
Summary by CodeRabbit
Bug Fixes