chore(PE-8643): hb patch system for primary names #438
Conversation
WalkthroughAdds a global HyperbeamSync table and new hb APIs to build/send primary-names patches and reset sync state; primary-names flows now record changes into HyperbeamSync; main.lua triggers hb.patchHyperbeamState() only when a balance patch occurred; tests updated/added to assert patch emission and payload shape. Changes
Sequence Diagram(s)sequenceDiagram
participant H as Handler
participant M as main.lua
participant PN as primary_names.lua
participant HB as hb.lua
H->>M: Event triggers handler
M->>M: shouldPatchHbState = false
M->>M: capture oldBalances
alt handler modifies state (not "continue")
M->>PN: primary-name operations (request/approve/remove/prune)
note right of PN #F0F4C3: PN updates HyperbeamSync.primaryNames maps
PN-->>HB: HyperbeamSync populated in-memory
M->>HB: hb.patchBalances(oldBalances)
HB->>HB: compute/send balance patch (if any)
alt balance patch occurred
M->>HB: hb.patchHyperbeamState()
HB->>HB: createPrimaryNamesPatch()
alt primary-names patch non-empty
HB->>HB: attach device metadata ([email protected]) and send patch
HB->>HB: resetHyperbeamSync()
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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: 3
🧹 Nitpick comments (2)
src/hb.lua (2)
41-58: Send balance patches only when something changedYou always include
ao.id, sonext(patchMessage.balances)is nevernil; patches go out even when nothing changed. Gate on the delta instead. Keep using globalBalancesand the single-arg signature.Apply (no behavior change to “protocol balance always included when sending”):
- -- 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 if there are affected addresses + if next(affectedBalancesAddresses) == nil then return {} else ao.send(patchMessage) endBased on learnings.
4-24: Minor: tidy HyperbeamSync docsThe comments say “addresses that have had name changes” for
names; keys are names, not addresses.Suggested:
- Update inline comments to reflect correct key types (
nameskeyed by name;ownersandrequestskeyed by address).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/hb.lua(2 hunks)src/main.lua(2 hunks)src/primary_names.lua(7 hunks)tests/primary.test.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-08T16:45:02.923Z
Learnt from: atticusofsparta
PR: ar-io/ar-io-network-process#435
File: src/main.lua:0-0
Timestamp: 2025-10-08T16:45:02.923Z
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/main.luasrc/hb.lua
📚 Learning: 2025-10-08T16:44:07.797Z
Learnt from: atticusofsparta
PR: ar-io/ar-io-network-process#435
File: src/hb.lua:0-0
Timestamp: 2025-10-08T16:44:07.797Z
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/main.luasrc/hb.lua
🧬 Code graph analysis (2)
tests/primary.test.mjs (1)
tests/helpers.mjs (3)
buyRecord(810-842)buyRecord(810-842)assertNoResultError(62-68)
src/main.lua (1)
src/utils.lua (1)
utils.deepCopy(393-476)
🔇 Additional comments (6)
src/primary_names.lua (5)
149-151: LGTM: request tracking on createMarking
HyperbeamSync.primaryNames.requests[initiator] = trueon async request creation is correct and minimal.
235-239: LGTM: comprehensive sync on set-from-requestTracking
names,owners, and clearingrequestsaligns with patch semantics.
289-293: LGTM: precise sync on single-name removalThese flags target the correct keys (
name,primaryName.owner, andrequests).
371-373: LGTM: sync on base-name cascade removalCorrectly flags each removed pair under names/owners.
436-438: LGTM: sync on request pruneMarking
requests[initiator] = trueensures patches reflect expiry.tests/primary.test.mjs (1)
686-1002: Tests align with patch semanticsThe new suite asserts
deviceandprimary-namespayloads across request/approve/remove flows. After fixingcreatePrimaryNamesPatchand the function call inmain.lua, these should pass.To run just this suite locally, filter on “hyperbeam patch”.
atticusofsparta
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #438 +/- ##
========================================
Coverage 91.36% 91.36%
========================================
Files 10 10
Lines 3218 3230 +12
========================================
+ Hits 2940 2951 +11
- Misses 278 279 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/globals.lua(1 hunks)src/hb.lua(1 hunks)src/main.lua(2 hunks)tests/patch-primary-names.test.mjs(1 hunks)tests/primary.test.mjs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main.lua
- src/hb.lua
- tests/primary.test.mjs
🧰 Additional context used
🧠 Learnings (2)
📓 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.923Z
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:45:02.923Z
Learnt from: atticusofsparta
PR: ar-io/ar-io-network-process#435
File: src/main.lua:0-0
Timestamp: 2025-10-08T16:45:02.923Z
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/globals.lua
🧬 Code graph analysis (1)
tests/patch-primary-names.test.mjs (1)
tests/primary.test.mjs (9)
totalTokenSupplyMemory(21-23)patchMessage(713-713)patchMessage(783-783)patchMessage(885-885)patchMessage(955-955)deviceTag(716-716)deviceTag(786-786)deviceTag(888-888)deviceTag(958-958)
⏰ 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). (2)
- GitHub Check: sdk
- GitHub Check: unit
🔇 Additional comments (3)
src/globals.lua (1)
15-27: LGTM: Clean state tracking structure.The HyperbeamSync initialization follows idiomatic Lua patterns with proper state preservation using the
oroperator. The nested structure with boolean flags is an efficient approach for tracking dirty state that needs synchronization. Type annotations add helpful documentation.tests/patch-primary-names.test.mjs (2)
14-68: LGTM: Comprehensive test setup.The test setup is well-structured and thorough:
- Properly initializes token supply and transfers balances to test addresses
- Creates an ArNS record owned by a different address than the requester, which correctly tests the primary name request flow
- Uses incremental timestamps to avoid timing conflicts
- Follows established testing patterns from the helper functions
71-104: LGTM: Thorough patch message validation.The patch message assertions are well-structured:
- Correctly searches backwards through messages to find the most recent patch with device tag "[email protected]"
- Validates the presence of the primary-names tag
- Thoroughly checks the patch data structure, including the requests map and the specific entry for the test caller
- The comment on line 92 about not JSON-encoding patch messages is a helpful implementation detail
The test provides good coverage for the primary name request patching functionality.
atticusofsparta
force-pushed
the
PE-8643-hb-patch-system
branch
from
3ca3ed0 to
9876f91
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/globals.lua(1 hunks)src/hb.lua(1 hunks)src/main.lua(2 hunks)tests/patch-primary-names.test.mjs(1 hunks)tests/primary.test.mjs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/hb.lua
- src/main.lua
- src/globals.lua
- tests/patch-primary-names.test.mjs
🧰 Additional context used
🧠 Learnings (1)
📓 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.923Z
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.
🧬 Code graph analysis (1)
tests/primary.test.mjs (3)
tests/patch-balances.test.mjs (6)
patchMessage(38-38)patchMessage(69-69)patchMessage(119-119)patchData(39-41)patchData(70-72)patchData(120-122)tests/helpers.mjs (3)
buyRecord(810-842)buyRecord(810-842)assertNoResultError(62-68)tests/patch-primary-names.test.mjs (2)
patchMessage(73-73)deviceTag(76-76)
⏰ 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/primary.test.mjs (3)
331-332: LGTM!The updated message count correctly accounts for the additional patch message emitted during primary name approval.
456-457: LGTM!The updated message count correctly accounts for the patch message emitted when the owner immediately approves their own primary name request.
567-568: LGTM!The updated message count correctly accounts for the patch message emitted when primary names are removed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/primary_names.lua (2)
259-261: Remove redundant tracking (already handled by removePrimaryName).Lines 259-261 track
namesandownersagain after callingremovePrimaryName(line 258), which already tracks all three maps (names, owners, requests) at lines 288-292. This creates duplicate tracking entries for the same changes.Apply this diff to remove the redundant tracking:
local removedPrimaryNameAndOwner = primaryNames.removePrimaryName(name, from) - -- track the changes in the hyperbeam sync - HyperbeamSync.primaryNames.names[name] = true - HyperbeamSync.primaryNames.owners[removedPrimaryNameAndOwner.owner] = true - table.insert(removedPrimaryNamesAndOwners, removedPrimaryNameAndOwner)
370-372: Remove redundant tracking (already handled by removePrimaryName).Lines 370-372 track
namesandownersafter callingremovePrimaryName(line 369), which already tracks all three maps at lines 288-292. This creates duplicate tracking entries.Apply this diff to remove the redundant tracking:
local removedName = primaryNames.removePrimaryName(nameData.name, nameData.owner) - -- track the changes in the hyperbeam sync - HyperbeamSync.primaryNames.names[nameData.name] = true - HyperbeamSync.primaryNames.owners[nameData.owner] = true table.insert(removedNames, removedName)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/primary_names.lua(7 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 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.923Z
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.
⏰ 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/primary_names.lua (4)
149-150: LGTM: Request tracking is correct.The HyperbeamSync tracking correctly captures the initiator when an asynchronous request is stored.
234-238: LGTM: Comprehensive tracking after state updates.All three maps (names, owners, requests) are correctly tracked after the internal state changes.
288-292: LGTM: Complete tracking in base removal function.The base removal function correctly tracks all three maps after state modifications.
435-437: LGTM: Pruning tracked correctly.The HyperbeamSync tracking correctly captures pruned requests after state modifications.
src/globals.lua
Outdated
|
|
||
| --[[ | ||
| HyperbeamSync is a table that is used to track changes to our lua state that need to be synced to the Hyperbeam. | ||
| the principal of using it is to set the key:value pairs that need to be synced, then |
There was a problem hiding this comment.
Nits: The principle and weird tabbing on the line below.
|
|
||
| -- set the primary name | ||
| local newPrimaryName = primaryNames.setPrimaryNameFromRequest(recipient, request, timestamp) | ||
|
|
There was a problem hiding this comment.
Nit: stray whitespace.
src/hb.lua
Outdated
|
|
||
| function hb.resetHyperbeamSync() | ||
| HyperbeamSync = { | ||
| balances = {}, |
Summary by CodeRabbit
New Features
Tests