-
Notifications
You must be signed in to change notification settings - Fork 4
fix(primary names hb): update createPrimaryNamesPatch to use PrimaryN… #444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ameInfo on the owner key
WalkthroughA new patch function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #444 +/- ##
========================================
Coverage 91.36% 91.36%
========================================
Files 10 10
Lines 3230 3230
========================================
Hits 2951 2951
Misses 279 279 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
patches/2025-11-06-primary-names-hb-sync-fix.lua (1)
9-71: Patch implementation correctly mirrors the updated function.The patch appropriately overrides the
createPrimaryNamesPatchfunction with logic identical to the updated version insrc/hb.lua, enabling hot-fix deployment.Note: The same concern about uni-directional iteration (flagged in
src/hb.lualines 57-60) applies here. If that verification reveals an issue, this patch will need updating as well.
🧹 Nitpick comments (1)
src/hb.lua (1)
35-40: Type annotations improve clarity.The explicit type annotations for
affectedPrimaryNamesAddressesand theownersfield enhance type safety and IDE support.Note: The type annotation on line 38 is redundant since line 35 already specifies
owners: table<string, PrimaryNameInfo>. You can remove line 38 if desired:local affectedPrimaryNamesAddresses = { names = {}, - ---@type table<string, PrimaryNameInfo> owners = {}, requests = {}, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
patches/2025-11-06-primary-names-hb-sync-fix.lua(1 hunks)src/hb.lua(3 hunks)src/primary_names.lua(0 hunks)
💤 Files with no reviewable changes (1)
- src/primary_names.lua
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: atticusofsparta
Repo: ar-io/ar-io-network-process PR: 442
File: src/hb.lua:7-27
Timestamp: 2025-11-03T22:41:58.658Z
Learning: In `src/hb.lua`, the `hb.createBalancesPatch()` function intentionally iterates both `Balances` and `HyperbeamSync.balances` in two separate loops to detect all changes (additions, modifications, and removals) for complete remote store synchronization. The `HyperbeamSync.balances` table holds a snapshot of balances before handlers run, so comparing in both directions ensures every altered address is included in the patch sent to Hyperbeam.
Learnt from: atticusofsparta
Repo: ar-io/ar-io-network-process PR: 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-11-03T22:41:58.658Z
Learnt from: atticusofsparta
Repo: ar-io/ar-io-network-process PR: 442
File: src/hb.lua:7-27
Timestamp: 2025-11-03T22:41:58.658Z
Learning: In `src/hb.lua`, the `hb.createBalancesPatch()` function intentionally iterates both `Balances` and `HyperbeamSync.balances` in two separate loops to detect all changes (additions, modifications, and removals) for complete remote store synchronization. The `HyperbeamSync.balances` table holds a snapshot of balances before handlers run, so comparing in both directions ensures every altered address is included in the patch sent to Hyperbeam.
Applied to files:
patches/2025-11-06-primary-names-hb-sync-fix.luasrc/hb.lua
📚 Learning: 2025-10-08T16:45:02.923Z
Learnt from: atticusofsparta
Repo: ar-io/ar-io-network-process PR: 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:
patches/2025-11-06-primary-names-hb-sync-fix.luasrc/hb.lua
📚 Learning: 2025-10-08T16:44:07.797Z
Learnt from: atticusofsparta
Repo: ar-io/ar-io-network-process PR: 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:
patches/2025-11-06-primary-names-hb-sync-fix.luasrc/hb.lua
🧬 Code graph analysis (2)
patches/2025-11-06-primary-names-hb-sync-fix.lua (1)
src/primary_names.lua (1)
primaryNames.getPrimaryNameDataWithOwnerFromAddress(310-322)
src/hb.lua (1)
src/primary_names.lua (1)
primaryNames.getPrimaryNameDataWithOwnerFromAddress(310-322)
⏰ 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/hb.lua (3)
3-3: Good addition of the primary names dependency.This import enables access to the enhanced owner data retrieval function.
59-59: Enhanced owner data format aligns with PR objective.Using
getPrimaryNameDataWithOwnerFromAddresscorrectly provides SDK-compatible owner information including theprocessIdfield, with proper fallback to{}for removals.
57-60: Uni-directional iteration appears intentional but uses different architecture than balances.The
createBalancesPatchuses a snapshot model storing previous values, enabling bidirectional comparison. ThecreatePrimaryNamesPatchuses a dirty flag model, iterating onlyHyperbeamSync.primaryNames.*entries marked astrueduring mutations.The dirty flag approach works because all mutations in
src/primary_names.luaexplicitly set the corresponding flags (lines 150, 236-238, 260-261, 290-292, 370-371, 436). However, this is less robust than the snapshot pattern—if tracking discipline breaks or state is modified outside tracked functions, changes could be missed.Consider either: (1) confirm this architectural difference is intentional, or (2) refactor
createPrimaryNamesPatchto match the snapshot comparison pattern used bycreateBalancesPatchfor consistency and resilience.patches/2025-11-06-primary-names-hb-sync-fix.lua (1)
1-8: Clear patch documentation.The header comment clearly explains the purpose and scope of this patch, aligning with the PR objective.
…ameInfo on the owner key
Summary by CodeRabbit