chore(PE-8188): add patch balances work #435
Conversation
WalkthroughAdds a standalone Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Owner as Owner
participant Client as Client
participant Runtime as src/main.lua
participant HB as src/hb.lua
participant AO as ao
rect rgba(220,235,255,0.45)
note over Client,Runtime: Event/action lifecycle with HB patch invocation
Client->>Runtime: Action X
Runtime->>Runtime: oldBalances = deepCopy(Balances)
Runtime->>Runtime: execute handler for Action X
Runtime->>HB: patchBalances(oldBalances)
HB->>HB: compute diffs vs current Balances
alt affected addresses
HB->>AO: ao.send({ Device: "[email protected]", balances: {...} })
HB-->>Runtime: affectedAddresses
else no changes
HB-->>Runtime: {}
end
Runtime-->>Client: Result (Messages may include patch)
end
rect rgba(220,255,220,0.4)
note over Owner,Runtime: Owner-triggered PatchHyperbeamBalances
Owner->>Runtime: PatchHyperbeamBalances
Runtime->>Runtime: oldBalances = deepCopy(Balances)
Runtime->>HB: patchBalances(oldBalances)
HB->>AO: ao.send(patch) -- if any affected balances
Runtime-->>Owner: PatchHyperbeamBalances-Notice
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-10-08T16:45:02.913ZApplied to files:
📚 Learning: 2025-10-08T16:44:07.775ZApplied to files:
🧬 Code graph analysis (1)src/main.lua (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (10)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #435 +/- ##
========================================
Coverage 91.34% 91.34%
========================================
Files 10 10
Lines 3213 3213
========================================
Hits 2935 2935
Misses 278 278 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (3)
src/vaults.lua (1)
236-260: Consider guarding the patch call when no addresses are affected.If no vaults are pruned,
affectedAddresseswill be empty, andbalances.patchBalanceswill send a message containing only the protocol balance. While not incorrect, this may result in unnecessary messages.Apply this diff to optimize:
local affectedAddresses = {} if not NextBalanceVaultsPruneTimestamp or currentTimestamp < NextBalanceVaultsPruneTimestamp then -- No known pruning work to do return {} end local prunedVaults = {} -- reset the next prune timestamp, below will populate it with the next prune timestamp minimum NextBalanceVaultsPruneTimestamp = nil -- note: use unsafe to avoid copying all the vaults, directly update the vaults table for owner, ownersVaults in pairs(vaults.getVaultsUnsafe()) do for id, nestedVault in pairs(ownersVaults) do if currentTimestamp >= nestedVault.endTimestamp then balances.increaseBalance(owner, nestedVault.balance) table.insert(affectedAddresses, owner) prunedVaults[id] = vaults.removeVault(owner, id) else vaults.scheduleNextVaultsPruning(nestedVault.endTimestamp) end end end - balances.patchBalances(affectedAddresses) + if #affectedAddresses > 0 then + balances.patchBalances(affectedAddresses) + end return prunedVaultssrc/gar.lua (1)
937-1076: Consider guarding the patch call when no gateways are affected.Similar to the vault pruning pattern, if no gateways are pruned,
affectedAddresseswill be empty, resulting in a message containing only the protocol balance.Apply this diff to optimize:
for _, gateway in pairs(result.prunedGateways) do table.insert(affectedAddresses, gateway) end - balances.patchBalances(affectedAddresses) + if #affectedAddresses > 0 then + balances.patchBalances(affectedAddresses) + end return resultsrc/balances.lua (1)
99-110: LGTM! Consider adding address deduplication for optimization.The function correctly validates inputs and constructs the patch message. The decision to always include the protocol balance ensures consistency.
For optimization, consider deduplicating addresses to avoid sending duplicate balance entries:
function balances.patchBalances(addresses) assert(type(addresses) == "table", "Addresses must be a table") --- For simplicity we always include the protocol balance in the patch message local patchMessage = { device = "[email protected]", balances = { [ao.id] = Balances[ao.id] or 0 } } + local seen = { [ao.id] = true } for _, address in pairs(addresses) do assert(type(address) == "string", "Address must be a string") - patchMessage.balances[address] = Balances[address] or 0 + if not seen[address] then + patchMessage.balances[address] = Balances[address] or 0 + seen[address] = true + end end ao.send(patchMessage) end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
handlers-that-modify-balances.md(1 hunks)src/balances.lua(1 hunks)src/gar.lua(2 hunks)src/main.lua(20 hunks)src/vaults.lua(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/vaults.lua (1)
src/balances.lua (1)
balances.patchBalances(100-110)
src/main.lua (3)
src/balances.lua (1)
balances.patchBalances(100-110)src/utils.lua (1)
utils.parsePaginationTags(72-88)src/arns.lua (1)
arns.getPaginatedRecords(180-192)
src/gar.lua (1)
src/balances.lua (1)
balances.patchBalances(100-110)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sdk
🔇 Additional comments (2)
src/main.lua (2)
788-790: Verify if recipient balance patch is necessary.The comment "might not need recipient here" raises a valid concern. In a vaulted transfer, the recipient's balance doesn't change immediately since tokens are locked in a vault. The recipient only receives the balance when the vault matures.
Consider whether patching the recipient's balance is appropriate at this point, or if it should only occur when the vault is unlocked during pruning (which already patches balances in
src/vaults.lua).
2471-2473: Consider patching delegator balance in addition to protocol balance.The
RedelegateStakehandler only patchesao.id(protocol balance), which receives the redelegation fee. However, the delegator's balance might also be affected in scenarios where stake is moved from a withdrawal vault.Verify whether the delegator's address should be included in the patch:
- balances.patchBalances({ ao.id }) + balances.patchBalances({ ao.id, msg.From })This depends on whether the delegator's direct balance changes during redelegation, or if only staked/vaulted balances are affected (which would be reflected in gateway state rather than the delegator's direct balance).
atticusofsparta
changed the title
src/balances.lua
Outdated
| patchMessage.balances[address] = Balances[address] or 0 | ||
| end | ||
|
|
||
| ao.send(patchMessage) |
There was a problem hiding this comment.
Need to refresh my memory on how large this can actually scale in HB...
src/gar.lua
Outdated
| for _, gateway in pairs(result.prunedGateways) do | ||
| table.insert(affectedAddresses, gateway) | ||
| end | ||
| balances.patchBalances(affectedAddresses) |
There was a problem hiding this comment.
This does not seem to be in the spirit of "do the patches at the end". I'd prefer we be calling a global function on balances to reset the affected balance addresses at handlers start, update the addresses via global call during routine operations, and then patch at the end of all handling presuming there have been no errors.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main.lua (1)
1407-1446: Missing affected balance tracking in CancelWithdrawal.The
CancelWithdrawalhandler modifies balances by restoring stake from withdrawal vaults back to operator or delegated stake, but doesn't mark any addresses as affected. Based on the pattern in other handlers, this should mark relevant addresses (e.g.,msg.Fromor the gateway operator) to ensure their balances are patched.Apply this diff to add affected balance tracking:
Send(msg, { Target = msg.From, Tags = { Action = ActionMap.CancelWithdrawal .. "-Notice", Address = gatewayAddress, ["Vault-Id"] = msg.Tags["Vault-Id"], }, Data = json.encode(updatedGateway), }) + AffectedBalancesAddresses[msg.From] = true + if gatewayAddress ~= msg.From then + AffectedBalancesAddresses[gatewayAddress] = true + end end)
♻️ Duplicate comments (1)
src/main.lua (1)
1057-1058: TODO: Funding plan affected addresses.The TODO comment notes that when funding plans draw from stakes or vaults at other gateways, those addresses should also be marked as affected for balance patching. This concern was previously raised.
Based on past reviews, this issue has been identified but remains unresolved.
🧹 Nitpick comments (3)
src/main.lua (3)
796-798: Consider removing recipient from affected tracking.The comment acknowledges uncertainty about marking the recipient. Since:
- The recipient's direct balance doesn't change (they receive a vault, not liquid balance)
- Vault expiration already marks the owner via
pruneVaults(src/vaults.lua:251)Marking the recipient here may be unnecessary. Consider removing line 798 unless there's a specific reason to patch the recipient's balance at vault creation time.
837-838: Evaluate necessity of marking vault recipient.Similar to
VaultedTransfer, the recipient's vault is removed but their liquid balance doesn't change. Only the controller's balance increases, so marking the recipient as affected may be unnecessary.
1813-1813: Minor: Unnecessary empty line addition.This empty line doesn't appear to improve readability in the context of the distribute handler.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignore(1 hunks)src/gar.lua(1 hunks)src/globals.lua(1 hunks)src/main.lua(21 hunks)src/vaults.lua(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/gar.lua
🔇 Additional comments (5)
src/globals.lua (1)
21-27: LGTM! Clean addition of balance tracking global.The new
AffectedBalancesAddressesglobal follows established patterns in the file, includes clear documentation, and uses appropriate type annotation for tracking affected wallet addresses.src/vaults.lua (1)
250-251: Correct tracking of affected balance.Appropriately marks the vault owner as affected after their balance increases during vault pruning. This ensures the owner's balance will be patched in the post-handler processing.
src/main.lua (3)
497-502: Well-placed balance patching logic.The post-handler processing correctly:
- Captures affected addresses before cleanup
- Sends patch messages via
balances.patchBalances()- Resets
AffectedBalancesAddressesto prevent state leakageThis ensures every handler's balance changes are synchronized with the HyperBEAM state.
691-692: Correct tracking of both transfer parties.Both the sender and recipient are appropriately marked as affected, ensuring both balance updates are propagated.
966-966: Verify protocol balance patching requirement.The
BuyNamehandler marksmsg.Fromas affected but notao.id(the protocol), even though protocol balance increases from the purchase. In contrast,RedelegateStake(line 2482) does markao.id.This inconsistency appears in other handlers that credit the protocol:
UpgradeName(line 997)ExtendLease(line 1023)IncreaseUndernameLimit(line 1057)If the protocol balance requires patching to HyperBEAM, these handlers should also mark
ao.idas affected.
atticusofsparta
force-pushed
the
implement-balance-patching
branch
from
6de66c4 to
43a7a28
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/balances.lua (1)
99-124: Add comprehensive documentation and consider error handling.The function logic correctly identifies affected addresses (additions, deletions, and modifications), but has some areas for improvement:
- Missing documentation: Add JSDoc-style comments documenting the parameters, return value, and purpose
- Redundant logic: Both loops may mark the same changed address twice (harmless but inefficient)
- Missing error handling:
ao.send(patchMessage)has no error handling—if it fails, the function still returns successfully- Scalability concern: As noted in the past review comment at line 122, consider the impact when
Balancesgrows very largeApply this diff to add documentation:
+--- Computes affected balance addresses and sends a patch message +--- @param oldBalances table<string, number> Snapshot of balances before handler execution +--- @param newBalances table<string, number> Current balances after handler execution +--- @return table<string, true> Set of affected addresses (keys are addresses, values are true) function balances.patchBalances(oldBalances, newBalances)Consider wrapping
ao.sendin pcall for error handling:local status, err = pcall(ao.send, patchMessage) if not status then -- Log or handle the error appropriately print("Warning: Failed to send patch message: " .. tostring(err)) end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/balances.lua(1 hunks)src/main.lua(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main.lua (3)
src/utils.lua (3)
utils.deepCopy(393-476)utils.getTableKeys(623-629)utils.parsePaginationTags(72-88)src/balances.lua (1)
balances.patchBalances(101-124)src/arns.lua (1)
arns.getPaginatedRecords(180-192)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sdk
🔇 Additional comments (3)
src/main.lua (3)
635-635: LGTM: Formatting improvements.The cosmetic changes (extra blank lines and formatting adjustments) improve code readability without affecting functionality.
Also applies to: 1150-1155, 1794-1794
499-502: Verify the patch message protocol format.The integration correctly computes affected addresses and adds them to events. However, the patch message sent to HB uses
device = "[email protected]"as a protocol identifier.Please verify:
- Is
"[email protected]"the correct device identifier for the receiving system?- Does the receiving system expect the structure
{ device: string, balances: table }?- Are there any size limits on the patch message that could be exceeded with large balance sets?
Based on learnings: There was a past concern about scalability ("how large this can actually scale in HB").
1045-1045: Clarify or remove TODO for ARIO balance patches (src/main.lua:1045).
The lingering comment suggests missing FUND-based balance patches for other ARIO locations, but no related commits or code changes were found. If this TODO is obsolete, please remove it; otherwise, specify which ARIO locations still need patching and track them appropriately.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/balances.lua (1)
94-94: The incorrect return type documentation persists.The return type annotation still claims the function returns a
table<string, boolean>, but the function actually returns a single boolean value (line 96). This issue was previously flagged and remains unresolved.Apply this diff to correct the documentation:
---- @return table<string, boolean> A table of addresses and whether they have a sufficient balance +--- @return boolean Whether the wallet has sufficient balance function balances.walletHasSufficientBalance(wallet, quantity) return Balances[wallet] ~= nil and Balances[wallet] >= quantity end
🧹 Nitpick comments (2)
src/balances.lua (2)
99-101: Add return type documentation.The function is missing a
@returnannotation to document what it returns.Apply this diff to add the return documentation:
---@param oldBalances table<string, number> A table of addresses and their balances ---@param newBalances table<string, number> A table of addresses and their balances +---@return table<string, boolean> A table of affected addresses function balances.patchBalances(oldBalances, newBalances)
105-114: Consider clarifying the dual-loop logic.The two loops serve complementary purposes: the first (105-109) catches changes and deletions from
oldBalances, while the second (110-114) catches additions innewBalances. This approach works correctly but could be clearer. Consider adding a brief comment explaining why both loops are necessary, or refactor to a single loop that iterates over the union of both address sets.Example clarification:
local affectedBalancesAddresses = {} + -- Check for changes and deletions in addresses that existed in oldBalances for address, _ in pairs(oldBalances) do if Balances[address] ~= oldBalances[address] then affectedBalancesAddresses[address] = true end end + -- Check for additions and changes in addresses present in newBalances for address, _ in pairs(newBalances) do if oldBalances[address] ~= newBalances[address] then affectedBalancesAddresses[address] = true end end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/balances.lua(1 hunks)src/main.lua(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main.lua
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sdk
🔇 Additional comments (1)
src/balances.lua (1)
120-120: Verify that Balances equals newBalances when this function is called.Line 120 uses the current
Balances[address]values to populate the patch message. This assumes thatBalancesequalsnewBalancesat the timepatchBalancesis invoked. If this assumption is violated, the patch could contain inconsistent or stale balance data.Confirm that the calling code in
src/main.luacapturesnewBalancesas a snapshot ofBalancesimmediately before callingpatchBalances, and that no other balance modifications occur between the snapshot and the function call.
.gitignore
Outdated
|
|
||
| mainnet/**/outputs/* | ||
| mainnet/outputs/* | ||
| ._* |
There was a problem hiding this comment.
What's this one all about?
There was a problem hiding this comment.
macos pointer file generation when working on an external harddrive
atticusofsparta
force-pushed
the
implement-balance-patching
branch
from
b9ab716 to
4d915e0
Compare
src/main.lua
Outdated
| msg.ioEvent:addField("RecipientFormatted", recipient) | ||
|
|
||
| local result = balances.transfer(recipient, msg.From, quantity, allowUnsafeAddresses) | ||
|
|
src/main.lua
Outdated
| }, | ||
| Data = json.encode(fundFrom and result or recordResult), | ||
| }) | ||
| -- TODO patch other ARIO locations based on FUND |
src/main.lua
Outdated
| ]] | ||
| -- | ||
| print("Ticking from " .. lastCreatedEpochIndex .. " to " .. targetCurrentEpochIndex) | ||
|
|
atticusofsparta
force-pushed
the
implement-balance-patching
branch
from
994a208 to
6af1b0d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/balances.lua (1)
124-129: Clarify the empty-patch check logic.The check on line 125 appears unreachable because line 119 always adds
ao.idtopatchMessage.balances, ensuring it's never empty. This means the branch returning{}on line 126 is dead code.Based on the comment on line 118 ("this also prevents us from sending an empty patch message"), it seems the intention is to always include the protocol balance. If so, consider removing this check entirely since it can never be true. Alternatively, if you want to avoid sending patches when only the protocol balance is present, check
next(affectedBalancesAddresses)instead.Apply this diff to remove the unreachable check:
--- For simplicity we always include the protocol balance in the patch message --- this also prevents us from sending an empty patch message and deleting the entire hyperbeam balances table local patchMessage = { device = "[email protected]", balances = { [ao.id] = Balances[ao.id] or 0 } } for address, _ in pairs(affectedBalancesAddresses) do patchMessage.balances[address] = Balances[address] or 0 end - -- only send the patch message if there are affected balances, otherwise we'll end up deleting the entire hyperbeam balances table - if next(patchMessage.balances) == nil then - return {} - else - ao.send(patchMessage) - end + -- Always send the patch message since ao.id is always included by design + ao.send(patchMessage) return affectedBalancesAddresses endsrc/main.lua (1)
2696-2708: LGTM on owner restriction.The owner-only assertion on line 2700 properly restricts this manual patching trigger. The handler provides a mechanism for full balance synchronization, which differs from the incremental patches sent automatically after each handler (line 502).
Consider adding a docstring comment above this handler explaining:
- This is a manual full-sync mechanism vs automatic incremental patches
- When an operator should use this action
- Potential performance implications for large balance tables
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/balances.lua(1 hunks)src/main.lua(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main.lua (4)
src/utils.lua (3)
utils.deepCopy(393-476)utils.parsePaginationTags(72-88)utils.hasMatchingTag(9-11)src/balances.lua (1)
balances.patchBalances(102-132)src/arns.lua (1)
arns.getPaginatedRecords(180-192)patches/2025-04-29-check-nil-transfer.lua (1)
Send(12-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sdk
…avoid circular require
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/balances.lua (1)
54-56: Critical: Calling removed function causes runtime error.Line 55 calls
balances.walletHasSufficientBalance(target, qty)but this function was removed from the module. This will cause a runtime error on any balance reduction operation.Apply this diff to inline the balance check:
function balances.reduceBalance(target, qty) - assert(balances.walletHasSufficientBalance(target, qty), "Insufficient balance") + assert(Balances[target] ~= nil and Balances[target] >= qty, "Insufficient balance") assert(qty > 0, "Quantity must be greater than 0")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/balances.lua(1 hunks)src/hb.lua(1 hunks)src/main.lua(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main.lua (3)
src/utils.lua (3)
utils.deepCopy(393-476)utils.parsePaginationTags(72-88)utils.hasMatchingTag(9-11)src/hb.lua (1)
hb.patchBalances(8-45)src/arns.lua (1)
arns.getPaginatedRecords(180-192)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sdk
🔇 Additional comments (1)
src/main.lua (1)
502-503: Verify: Are no-op patches handled efficiently?The code sends a patch after every handler execution (line 503), even if balances haven't changed. While
hb.patchBalancesshould detect no differences and avoid sending empty patches, confirm that this doesn't create unnecessary overhead for read-only handlers.Since the
hb.patchBalancesfunction returns an empty table when there are no affected addresses (line 39 in src/hb.lua), and this return value isn't used here, confirm that:
- Sending a patch message with only
ao.id(which is always included) is acceptable for unchanged balance states- The Hyperbeam receiver handles these minimal patches efficiently
src/hb.lua
Outdated
| function hb.patchBalances(oldBalances, newBalances) | ||
| assert(type(oldBalances) == "table", "Old balances must be a table") | ||
| assert(type(newBalances) == "table", "New balances must be a table") | ||
| local affectedBalancesAddresses = {} | ||
| for address, _ in pairs(oldBalances) do | ||
| if Balances[address] ~= oldBalances[address] then | ||
| affectedBalancesAddresses[address] = true | ||
| end | ||
| end | ||
| for address, _ in pairs(newBalances) do | ||
| if oldBalances[address] ~= newBalances[address] then | ||
| affectedBalancesAddresses[address] = true | ||
| end | ||
| end | ||
|
|
||
| --- For simplicity we always include the protocol balance in the patch message | ||
| --- this also prevents us from sending an empty patch message and deleting the entire hyperbeam balances table\ | ||
| local supplyStats = token.calculateTotalSupply() | ||
| local patchMessage = { | ||
| device = "[email protected]", | ||
| balances = { [ao.id] = Balances[ao.id] or 0 }, | ||
| info = { | ||
| supply = tostring(supplyStats.totalSupply), | ||
| }, | ||
| } | ||
| for address, _ in pairs(affectedBalancesAddresses) do | ||
| patchMessage.balances[address] = Balances[address] or 0 | ||
| end | ||
|
|
||
| -- only send the patch message if there are affected balances, otherwise we'll end up deleting the entire hyperbeam balances table | ||
| if next(patchMessage.balances) == nil then | ||
| return {} | ||
| else | ||
| ao.send(patchMessage) | ||
| end | ||
|
|
||
| return affectedBalancesAddresses |
There was a problem hiding this comment.
Use the newBalances parameter consistently instead of the global Balances.
The function accepts newBalances as a parameter but inconsistently uses the global Balances variable in several places (lines 13, 28, 34). This makes the code confusing and the parameter misleading. While it works because newBalances and Balances refer to the same table in practice, using the parameter consistently improves clarity and makes the function's dependencies explicit.
Apply this diff to use the parameter consistently:
function hb.patchBalances(oldBalances, newBalances)
assert(type(oldBalances) == "table", "Old balances must be a table")
assert(type(newBalances) == "table", "New balances must be a table")
local affectedBalancesAddresses = {}
for address, _ in pairs(oldBalances) do
- if Balances[address] ~= oldBalances[address] then
+ if newBalances[address] ~= oldBalances[address] then
affectedBalancesAddresses[address] = true
end
end
for address, _ in pairs(newBalances) do
if oldBalances[address] ~= newBalances[address] then
affectedBalancesAddresses[address] = true
end
end
--- For simplicity we always include the protocol balance in the patch message
--- this also prevents us from sending an empty patch message and deleting the entire hyperbeam balances table\
local supplyStats = token.calculateTotalSupply()
local patchMessage = {
device = "[email protected]",
- balances = { [ao.id] = Balances[ao.id] or 0 },
+ balances = { [ao.id] = newBalances[ao.id] or 0 },
info = {
supply = tostring(supplyStats.totalSupply),
},
}
for address, _ in pairs(affectedBalancesAddresses) do
- patchMessage.balances[address] = Balances[address] or 0
+ patchMessage.balances[address] = newBalances[address] or 0
end🤖 Prompt for AI Agents
In src/hb.lua around lines 8 to 44, the function uses the global Balances in
three places instead of the newBalances parameter; replace Balances[address]
comparisons in the first loop with newBalances[address], set the protocol entry
in patchMessage.balances to newBalances[ao.id] (or 0) instead of
Balances[ao.id], and assign patchMessage.balances[address] =
newBalances[address] (or 0) in the final loop so the function consistently uses
the provided newBalances parameter.
There was a problem hiding this comment.
We moved to use the global Balances here and just pass in oldBalances instead
There was a problem hiding this comment.
@atticusofsparta, understood! Using the global Balances directly and only passing oldBalances makes sense and simplifies the function signature. This design makes it clear that the function operates on the current state of Balances.
✏️ Learnings added
Learnt from: atticusofsparta
PR: ar-io/ar-io-network-process#435
File: src/hb.lua:0-0
Timestamp: 2025-10-08T16:44:07.775Z
Learning: In `src/hb.lua`, the `hb.patchBalances()` function intentionally uses the global `Balances` variable directly and only accepts `oldBalances` as a parameter, rather than accepting both old and new balances as parameters.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
src/main.lua
Outdated
| addEventingHandler( | ||
| ActionMap.PatchHyperbeamBalances, | ||
| utils.hasMatchingTag("Action", ActionMap.PatchHyperbeamBalances), | ||
| function(msg) | ||
| assert(msg.From == Owner, "Only the owner can trigger " .. ActionMap.PatchHyperbeamBalances) | ||
| local patchMessage = { device = "[email protected]", balances = utils.deepCopy(Balances) } | ||
| ao.send(patchMessage) | ||
| return Send(msg, { | ||
| Target = msg.From, | ||
| Action = ActionMap.PatchHyperbeamBalances .. "-Notice", | ||
| }) | ||
| end | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify: Should the manual patch include supply information?
The automatic patch in hb.patchBalances includes info.supply (line 30 in src/hb.lua), but this manual owner-triggered patch on line 2702 does not. Confirm whether the manual patch should also include the total supply information for consistency with the automatic patching mechanism.
Run the following script to check if there are any other patch message constructions in the codebase:
🏁 Script executed:
#!/bin/bash
# Search for other patch message constructions to verify consistency
rg -n "device.*patch" --type lua -C 2Length of output: 2241
Include total supply in manual patch
The owner-triggered patch in src/main.lua omits the info.supply field that hb.patchBalances sets via token.calculateTotalSupply(). Add info = { supply = token.calculateTotalSupply() } to the manual patchMessage for consistency.
🤖 Prompt for AI Agents
In src/main.lua around lines 2697 to 2709, the owner-triggered manual
patchMessage omits the info.supply field that hb.patchBalances would set; update
the construction of patchMessage to include an info table with supply =
token.calculateTotalSupply() so the message mirrors hb.patchBalances (e.g., set
patchMessage = { device = "[email protected]", balances = utils.deepCopy(Balances), info
= { supply = token.calculateTotalSupply() } }). Ensure
token.calculateTotalSupply() is available in scope and keep deepCopy(Balances)
as-is.
There was a problem hiding this comment.
this is deliberate, we set it once in the patch message to initialize hb state
There was a problem hiding this comment.
@atticusofsparta, thank you for clarifying! I understand now—the manual Patch-Hyperbeam-Balances handler is intended for one-time initialization of the Hyperbeam state, while the automatic patches (via hb.patchBalances()) handle ongoing state synchronization and include the supply information. That distinction makes sense.
✏️ Learnings added
Learnt from: atticusofsparta
PR: ar-io/ar-io-network-process#435
File: src/main.lua:0-0
Timestamp: 2025-10-08T16:45:02.913Z
Learning: In `src/main.lua`, the owner-triggered manual `Patch-Hyperbeam-Balances` handler deliberately omits the `info.supply` field that `hb.patchBalances()` includes. The manual patch is for one-time initialization of Hyperbeam state, while automatic patches handle ongoing state synchronization with supply information.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: atticusofsparta
PR: ar-io/ar-io-network-process#435
File: src/hb.lua:0-0
Timestamp: 2025-10-08T16:44:07.775Z
Learning: In `src/hb.lua`, the `hb.patchBalances()` function intentionally uses the global `Balances` variable directly and only accepts `oldBalances` as a parameter, rather than accepting both old and new balances as parameters.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (2)
78-78: MoveprocessOwnerdeclaration before thehandlefunction.The
handlefunction referencesprocessOwner(line 68), but the constant is defined here. While this works due to hoisting, declaringprocessOwnerbeforehandleimproves readability and follows the convention of defining dependencies before use.Apply this diff to move the declaration:
+const processOwner = 'My21NOHZyyeQG0t0yANsWjRakNDM7CJvd8urtdMLEDE'; + /** * * @param {{And remove it from line 78:
-const processOwner = 'My21NOHZyyeQG0t0yANsWjRakNDM7CJvd8urtdMLEDE'; - describe('2025-10-07-hb-balances-patch', () => {
92-92: Remove debuggingconsole.dirstatement.This appears to be a debugging artifact. Remove it or replace with meaningful assertions.
Apply this diff:
memory: wasmMemory, }); - - console.dir(rest, { depth: null }); const balances = await handle({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs(1 hunks)tests/utils.mjs(2 hunks)tools/constants.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/utils.mjs
🧰 Additional context used
🧬 Code graph analysis (1)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (2)
tools/constants.mjs (10)
STUB_TIMESTAMP(16-16)STUB_TIMESTAMP(16-16)STUB_BLOCK_HEIGHT(17-17)STUB_BLOCK_HEIGHT(17-17)STUB_HASH_CHAIN(20-20)STUB_HASH_CHAIN(20-20)DEFAULT_HANDLE_OPTIONS(60-70)DEFAULT_HANDLE_OPTIONS(60-70)AO_LOADER_HANDLER_ENV(22-37)AO_LOADER_HANDLER_ENV(22-37)tests/utils.mjs (3)
createAosLoader(15-42)handle(19-19)assertNoResultError(44-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sdk
🔇 Additional comments (1)
tools/constants.mjs (1)
24-34: LGTM!The explicit Process.Id, Module.Id, and structured Tags array improve clarity and align with the test setup that uses these same identifiers.
| it('should eval the patch file', async () => { | ||
| const { memory: evalPatchMemory, ...rest } = await handle({ | ||
| options: { | ||
| From: processOwner, | ||
| Owner: processOwner, | ||
| Tags: [{ name: 'Action', value: 'Eval' }], | ||
| Data: patchFile, | ||
| }, | ||
| memory: wasmMemory, | ||
| }); | ||
|
|
||
| console.dir(rest, { depth: null }); | ||
|
|
||
| const balances = await handle({ | ||
| options: { | ||
| From: processOwner, | ||
| Owner: processOwner, | ||
| Tags: [{ name: 'Action', value: 'Balances' }], | ||
| }, | ||
| memory: evalPatchMemory, | ||
| }); | ||
| console.dir(balances, { depth: null }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add assertions to validate patch evaluation.
The test only logs output via console.dir but doesn't assert expected behavior. Consider adding assertions to verify:
- The patch evaluates without errors
- Expected balance state after patch application
- Specific accounts have expected balances
Example assertions to add:
const balances = await handle({
options: {
From: processOwner,
Owner: processOwner,
Tags: [{ name: 'Action', value: 'Balances' }],
},
memory: evalPatchMemory,
});
- console.dir(balances, { depth: null });
+
+ // Assert balances message structure
+ assert.ok(balances.Messages?.length > 0, 'Should return balance messages');
+
+ // Parse and validate balance data
+ const balanceData = JSON.parse(balances.Messages[0].Data);
+ assert.ok(typeof balanceData === 'object', 'Balance data should be an object');
+ // Add specific balance assertions based on expected patch behavior📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should eval the patch file', async () => { | |
| const { memory: evalPatchMemory, ...rest } = await handle({ | |
| options: { | |
| From: processOwner, | |
| Owner: processOwner, | |
| Tags: [{ name: 'Action', value: 'Eval' }], | |
| Data: patchFile, | |
| }, | |
| memory: wasmMemory, | |
| }); | |
| console.dir(rest, { depth: null }); | |
| const balances = await handle({ | |
| options: { | |
| From: processOwner, | |
| Owner: processOwner, | |
| Tags: [{ name: 'Action', value: 'Balances' }], | |
| }, | |
| memory: evalPatchMemory, | |
| }); | |
| console.dir(balances, { depth: null }); | |
| }); | |
| it('should eval the patch file', async () => { | |
| const { memory: evalPatchMemory, ...rest } = await handle({ | |
| options: { | |
| From: processOwner, | |
| Owner: processOwner, | |
| Tags: [{ name: 'Action', value: 'Eval' }], | |
| Data: patchFile, | |
| }, | |
| memory: wasmMemory, | |
| }); | |
| console.dir(rest, { depth: null }); | |
| const balances = await handle({ | |
| options: { | |
| From: processOwner, | |
| Owner: processOwner, | |
| Tags: [{ name: 'Action', value: 'Balances' }], | |
| }, | |
| memory: evalPatchMemory, | |
| }); | |
| // Assert balances message structure | |
| assert.ok(balances.Messages?.length > 0, 'Should return balance messages'); | |
| // Parse and validate balance data | |
| const balanceData = JSON.parse(balances.Messages[0].Data); | |
| assert.ok(typeof balanceData === 'object', 'Balance data should be an object'); | |
| // Add specific balance assertions based on expected patch behavior | |
| }); |
🤖 Prompt for AI Agents
In tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs around lines
81 to 103, the test only logs results and lacks assertions; add explicit
assertions to verify the patch evaluation and resulting balances: assert that
evalPatchMemory (or the handler response) is defined and contains no error
field, assert the balances response is defined and shaped as expected (e.g., an
object/array), and assert specific account balances changed to the expected
values (use strict equality or deep equality assertions from your test
framework); include clear failure messages for each assertion so the test fails
when the patch does not apply or balances are incorrect.
There was a problem hiding this comment.
will come back to these live memory tests later
There was a problem hiding this comment.
@atticusofsparta, understood! Would you like me to create a follow-up issue to track adding proper assertions to the live memory tests?
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (1)
144-167: Add assertions to validate patch evaluation.The test only logs output via
console.logandconsole.dirbut doesn't assert expected behavior. Add assertions to verify:
- The patch evaluates without errors
- Expected balance state after patch application
- Specific accounts have expected balances
Example assertions to add:
const balances = await handle({ options: { From: processOwner, Owner: processOwner, Tags: [{ name: 'Action', value: 'Balances' }], }, - memory: null, + memory: evalPatchMemory, }); - console.dir(balances, { depth: null }); + // Assert balances message structure + assert.ok(balances.Messages?.length > 0, 'Should return balance messages'); + + // Parse and validate balance data + const balanceData = JSON.parse(balances.Messages[0].Data); + assert.ok(typeof balanceData === 'object', 'Balance data should be an object'); + assert.ok(Object.keys(balanceData).length > 0, 'Should have at least one balance entry');
🧹 Nitpick comments (1)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (1)
34-37: Remove commented-out code.The commented-out
lua: patchFileparameter on line 36 should be removed if it's no longer needed, or uncommented if it's part of the intended test setup.Apply this diff to remove the comment:
const { handle: originalHandle, memory } = await createAosLoader({ wasm, - // lua: patchFile, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (2)
tools/constants.mjs (14)
STUB_BLOCK_HEIGHT(17-17)STUB_BLOCK_HEIGHT(17-17)STUB_HASH_CHAIN(20-20)STUB_HASH_CHAIN(20-20)DEFAULT_HANDLE_OPTIONS(60-70)DEFAULT_HANDLE_OPTIONS(60-70)AO_LOADER_HANDLER_ENV(22-37)AO_LOADER_HANDLER_ENV(22-37)STUB_ADDRESS(10-10)STUB_ADDRESS(10-10)INITIAL_OPERATOR_STAKE(13-13)INITIAL_OPERATOR_STAKE(13-13)STUB_TIMESTAMP(16-16)STUB_TIMESTAMP(16-16)tests/utils.mjs (3)
createAosLoader(15-42)handle(19-19)assertNoResultError(44-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sdk
🔇 Additional comments (3)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (3)
49-86: LGTM!The handle wrapper correctly merges options with defaults, sets required fields, and conditionally validates results. The implementation is clean and maintainable.
92-120: LGTM!The transfer helper is well-structured with proper error handling and an efficient early return for zero-quantity transfers.
122-141: LGTM!The getBalances helper correctly retrieves and parses balance data with appropriate error handling.
src/main.lua
Outdated
|
|
||
| addEventingHandler( | ||
| ActionMap.PatchHyperbeamBalances, | ||
| utils.hasMatchingTag("Action", ActionMap.PatchHyperbeamBalances), |
There was a problem hiding this comment.
Wrap this handler with Handlers.continue
There was a problem hiding this comment.
I tried Handlers.utils.continue but it didnt perform as expected, so i just did a manual pattern function. Our tests cover it well.
| }); | ||
| const startMemory = memory; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Might want to note why we're making another handle helper function here (i.e. needing to supply target in current fixture's module version).
| memory: wasmMemory, | ||
| }); | ||
|
|
||
| console.log(evalPatchMemory); |
There was a problem hiding this comment.
Remove logs if not needed and add some assertions.
There was a problem hiding this comment.
will come back to revamp these live memory tests later
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (2)
163-163: Fix memory parameter to test patched state.The test passes
memory: undefinedinstead ofevalPatchMemory, causing the Balances action to operate on unpatched state rather than the patched state. This makes the test ineffective.Apply this diff to fix:
- memory: undefined, + memory: evalPatchMemory,
155-166: Add assertions to validate patch behavior.The test only logs output via
console.logandconsole.dirwithout verifying expected behavior. Add assertions to confirm:
- The patch evaluates without errors
- Balances reflect expected state after patching
- Specific accounts have expected balances
Example assertions to add:
- console.log(evalPatchMemory); + assert.ok(evalPatchMemory, 'Patch should evaluate successfully'); const balances = await handle({ options: { From: processOwner, Owner: processOwner, Tags: [{ name: 'Action', value: 'Balances' }], }, - memory: undefined, + memory: evalPatchMemory, }); - console.dir(balances, { depth: null }); + assert.ok(balances.Messages?.length > 0, 'Should return balance messages'); + const balanceData = JSON.parse(balances.Messages[0].Data); + assert.ok(typeof balanceData === 'object', 'Balance data should be an object'); + // Add specific balance assertions based on expected patch behavior
🧹 Nitpick comments (1)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (1)
169-329: Remove commented-out test code.The large commented-out block duplicates tests that appear to be implemented in
tests/patch-balances.test.mjs. Either remove this dead code or uncomment and implement the tests if they're needed here.Apply this diff to remove dead code:
- // describe('hyperbeam patch balances', async () => { - // it('should handle sending a patch to a newly created address', async () => { - // ... - // }); - // });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/balances.lua(0 hunks)src/main.lua(6 hunks)tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs(1 hunks)tests/patch-balances.test.mjs(1 hunks)
💤 Files with no reviewable changes (1)
- src/balances.lua
🧰 Additional context used
🧬 Code graph analysis (3)
src/main.lua (3)
src/utils.lua (2)
utils.deepCopy(393-476)utils.parsePaginationTags(72-88)src/hb.lua (1)
hb.patchBalances(7-41)src/arns.lua (1)
arns.getPaginatedRecords(180-192)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (2)
tools/constants.mjs (14)
STUB_BLOCK_HEIGHT(17-17)STUB_BLOCK_HEIGHT(17-17)STUB_HASH_CHAIN(20-20)STUB_HASH_CHAIN(20-20)DEFAULT_HANDLE_OPTIONS(60-70)DEFAULT_HANDLE_OPTIONS(60-70)AO_LOADER_HANDLER_ENV(22-37)AO_LOADER_HANDLER_ENV(22-37)STUB_ADDRESS(10-10)STUB_ADDRESS(10-10)INITIAL_OPERATOR_STAKE(13-13)INITIAL_OPERATOR_STAKE(13-13)STUB_TIMESTAMP(16-16)STUB_TIMESTAMP(16-16)tests/utils.mjs (3)
createAosLoader(15-42)handle(19-19)assertNoResultError(44-49)
tests/patch-balances.test.mjs (1)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (3)
transfer(92-120)getBalances(122-141)getBalances(122-141)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sdk
🔇 Additional comments (4)
src/main.lua (3)
477-482: LGTM! Good optimization for balance snapshot.The conditional deep-copy ensures balances are only captured for the final handler in a chain, avoiding redundant snapshots for intermediate handlers that return "continue".
506-509: LGTM! Patch call is properly guarded.The patch is only sent when oldBalances was captured, maintaining consistency with the conditional snapshot logic.
2703-2715: Verify: Should manual patch match automatic patch behavior?The manual owner-triggered patch sends all balances via
utils.deepCopy(Balances), whereas the automatic patch inhb.patchBalances(lines 6-40 in src/hb.lua) sends only affected balances plus the protocol balance.Is this intentional? If manual patches need a full refresh, the current approach is fine. Otherwise, consider reusing
hb.patchBalances(Balances, Balances)to ensure consistency.tests/patch-balances.test.mjs (1)
1-169: LGTM! Comprehensive test coverage for balance patching.The test cases effectively cover:
- Patch creation for newly created addresses
- Patch behavior when draining an address
- Patch handling when addresses are removed from balances
The assertions validate correct patch data in each scenario.
…set message length numbers, anchors, and refs
atticusofsparta
force-pushed
the
implement-balance-patching
branch
from
2c7fbbc to
e098651
Compare
There was a problem hiding this comment.
this is the actual module the mainnet ario process uses
src/hb.lua
Outdated
| local hb = {} | ||
|
|
||
| ---@param oldBalances table<string, number> A table of addresses and their balances | ||
| ---@param newBalances table<string, number> A table of addresses and their balances |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (1)
161-173: Add assertions; remove logs in smoke testReplace
console.dircalls with minimal assertions to validate patch eval and balances shape.
🧹 Nitpick comments (5)
src/hb.lua (3)
23-29: Include total supply in automatic patch (if available without cycles)Auto patches typically include
info.supplyfor consumers. Recommend adding it here iftoken.calculateTotalSupply()is accessible without reintroducing circular deps.Based on learnings
- local patchMessage = { + -- include total supply for HB consumers (optional but recommended) + local supplyStats = token and token.calculateTotalSupply and token.calculateTotalSupply() + local patchMessage = { device = "[email protected]", balances = { [ao.id] = Balances[ao.id] or 0 }, + info = supplyStats and { supply = tostring(supplyStats.totalSupply) } or nil, }If referencing
tokenhere would create a cycle, alternatively compute supply in the caller and add it to the message before sending.
7-8: Defensive guard for Balances tableAdd a quick assert to fail fast if
Balancesis unset.function hb.patchBalances(oldBalances) assert(type(oldBalances) == "table", "Old balances must be a table") + assert(type(Balances) == "table", "Balances must be a table")
20-22: Comment nitRemove the stray trailing backslash and align the comment with the corrected gating condition.
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (2)
71-72: Avoid hardcoded IDs; use theprocessIdconstantPrevents drift if the ID changes in one place.
- Target: 'qNvAoz0TgcH7DMg8BCVn8jF32QH5L6T29VjHxhHqqGE', + Target: processId, @@ - Id: 'qNvAoz0TgcH7DMg8BCVn8jF32QH5L6T29VjHxhHqqGE', + Id: processId,Also applies to: 77-78
115-121: Normalize tag values to stringsAo tags are conventionally strings. Convert
QuantityandCastto strings to avoid implicit coercion issues.Tags: [ { name: 'Action', value: 'Transfer' }, { name: 'Recipient', value: recipient }, - { name: 'Quantity', value: quantity }, - { name: 'Cast', value: cast }, + { name: 'Quantity', value: String(quantity) }, + { name: 'Cast', value: String(cast) }, ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/hb.lua(1 hunks)tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs(1 hunks)tools/constants.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/constants.mjs
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: atticusofsparta
PR: ar-io/ar-io-network-process#435
File: src/main.lua:0-0
Timestamp: 2025-10-08T16:45:02.913Z
Learning: In `src/main.lua`, the owner-triggered manual `Patch-Hyperbeam-Balances` handler deliberately omits the `info.supply` field that `hb.patchBalances()` includes. The manual patch is for one-time initialization of Hyperbeam state, while automatic patches handle ongoing state synchronization with supply information.
📚 Learning: 2025-10-08T16:44:07.775Z
Learnt from: atticusofsparta
PR: ar-io/ar-io-network-process#435
File: src/hb.lua:0-0
Timestamp: 2025-10-08T16:44:07.775Z
Learning: In `src/hb.lua`, the `hb.patchBalances()` function intentionally uses the global `Balances` variable directly and only accepts `oldBalances` as a parameter, rather than accepting both old and new balances as parameters.
Applied to files:
src/hb.lua
📚 Learning: 2025-10-08T16:45:02.913Z
Learnt from: atticusofsparta
PR: ar-io/ar-io-network-process#435
File: src/main.lua:0-0
Timestamp: 2025-10-08T16:45:02.913Z
Learning: In `src/main.lua`, the owner-triggered manual `Patch-Hyperbeam-Balances` handler deliberately omits the `info.supply` field that `hb.patchBalances()` includes. The manual patch is for one-time initialization of Hyperbeam state, while automatic patches handle ongoing state synchronization with supply information.
Applied to files:
src/hb.lua
🧬 Code graph analysis (1)
tests/code-patches/2025-10-07-hb-balances-patch/index.test.mjs (2)
tools/constants.mjs (14)
STUB_BLOCK_HEIGHT(17-17)STUB_BLOCK_HEIGHT(17-17)STUB_HASH_CHAIN(20-20)STUB_HASH_CHAIN(20-20)DEFAULT_HANDLE_OPTIONS(60-70)DEFAULT_HANDLE_OPTIONS(60-70)AO_LOADER_HANDLER_ENV(22-37)AO_LOADER_HANDLER_ENV(22-37)STUB_ADDRESS(10-10)STUB_ADDRESS(10-10)INITIAL_OPERATOR_STAKE(13-13)INITIAL_OPERATOR_STAKE(13-13)STUB_TIMESTAMP(16-16)STUB_TIMESTAMP(16-16)tests/utils.mjs (3)
createAosLoader(15-37)handle(19-19)assertNoResultError(39-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sdk
| -- only send the patch message if there are affected balances, otherwise we'll end up deleting the entire hyperbeam balances table | ||
| if next(patchMessage.balances) == nil then | ||
| return {} | ||
| else | ||
| ao.send(patchMessage) | ||
| end |
There was a problem hiding this comment.
Send gating is ineffective; patch always sent due to protocol entry
Because patchMessage.balances always contains [ao.id], next(patchMessage.balances) is never nil. Gate on affectedBalancesAddresses instead to avoid no-op patches.
Apply this diff:
- -- only send the patch message if there are affected balances, otherwise we'll end up deleting the entire hyperbeam balances table
- if next(patchMessage.balances) == nil then
+ -- only send when there are actual balance changes; avoid emitting no-op patches
+ if next(affectedBalancesAddresses) == nil then
return {}
else
ao.send(patchMessage)
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -- only send the patch message if there are affected balances, otherwise we'll end up deleting the entire hyperbeam balances table | |
| if next(patchMessage.balances) == nil then | |
| return {} | |
| else | |
| ao.send(patchMessage) | |
| end | |
| -- only send when there are actual balance changes; avoid emitting no-op patches | |
| if next(affectedBalancesAddresses) == nil then | |
| return {} | |
| else | |
| ao.send(patchMessage) | |
| end |
🤖 Prompt for AI Agents
In src/hb.lua around lines 31 to 36, the current check uses
next(patchMessage.balances) which is always non-nil because
patchMessage.balances contains [ao.id]; change the gate to check
next(affectedBalancesAddresses) instead and only call ao.send(patchMessage) when
affectedBalancesAddresses is non-empty (return {} when it's nil/empty) so no-op
patches are not sent; ensure affectedBalancesAddresses is a table in scope
before checking.
Summary by CodeRabbit
New Features
Behavior Changes
Tests / Chores