feat: track Handshake name hashes #451
Conversation
📝 WalkthroughWalkthroughThis PR adds handshake name storage and lookup to the state layer and integrates those calls into the handshake indexer. New Badger key prefixes were introduced: Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/indexer/handshake.go(2 hunks)internal/state/state.go(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
internal/indexer/handshake.go (2)
internal/handshake/covenant.go (8)
OpenCovenant(204-207)OpenCovenant(209-209)ClaimCovenant(161-168)ClaimCovenant(170-170)RegisterCovenant(327-332)RegisterCovenant(334-334)UpdateCovenant(366-370)UpdateCovenant(372-372)internal/state/state.go (1)
GetState(371-373)
🪛 GitHub Actions: golangci-lint
internal/state/state.go
[error] 342-342: golangci-lint: File is not properly formatted (gofumpt).
🪛 GitHub Check: lint
internal/state/state.go
[failure] 343-343:
unnecessary trailing newline (whitespace)
[failure] 342-342:
File is not properly formatted (gofumpt)
⏰ 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: cubic · AI code reviewer
- GitHub Check: Analyze (go)
🔇 Additional comments (10)
internal/state/state.go (5)
11-11: LGTM!The
crypto/sha3import is correctly added to support SHA-3-256 hashing of handshake names.
31-33: LGTM!The new key prefix constants provide clear namespacing for Badger storage keys, improving organization and reducing collision risk.
229-235: LGTM!The record key formatting correctly adopts the new
recordKeyPrefixconstant while preserving the existing logic.
257-257: LGTM!The domain tracking key correctly uses
domainKeyPrefixandfmt.Appendffor improved efficiency.
298-304: LGTM!The key prefix construction correctly uses
recordKeyPrefixand efficientfmt.Appendfformatting.internal/indexer/handshake.go (5)
16-16: LGTM!The
internal/stateimport is correctly added to support handshake name storage and retrieval.
116-119: LGTM!The OpenCovenant handling correctly stores the handshake name for future lookups and propagates errors appropriately.
120-123: LGTM!The ClaimCovenant handling correctly stores the handshake name for future lookups and propagates errors appropriately.
124-129: LGTM!The RegisterCovenant handling correctly retrieves the handshake name by its hash for logging, with appropriate error propagation.
130-135: LGTM!The UpdateCovenant handling correctly retrieves the handshake name by its hash for logging, with appropriate error propagation.
Signed-off-by: Aurora Gaffney <aurora@blinklabs.io>
agaffney
force-pushed
the
feat/handshake-storage
branch
from
fd52532 to
ed93565
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
internal/state/state.go (2)
334-347: Hash encoding fix confirmed; consider minor simplification.The critical hash encoding issue from the previous review has been correctly addressed by using
%xformat specifier for hex encoding. The implementation is now correct.Optionally simplify the error handling:
func (s *State) AddHandshakeName(name string) error { nameHash := sha3.Sum256([]byte(name)) nameHashKey := fmt.Sprintf("%s%x", handshakeNameHashKeyPrefix, nameHash) - err := s.db.Update(func(txn *badger.Txn) error { + return s.db.Update(func(txn *badger.Txn) error { return txn.Set( []byte(nameHashKey), []byte(name), ) }) - if err != nil { - return err - } - return nil }
349-368: Hash encoding fix confirmed; consider minor simplification.The critical hash encoding issue from the previous review has been correctly addressed by using
%xformat specifier. The lookup logic is correct.Optionally simplify the error handling:
func (s *State) GetHandshakeNameByHash(nameHash []byte) (string, error) { var ret string nameHashKey := fmt.Sprintf("%s%x", handshakeNameHashKeyPrefix, nameHash) err := s.db.View(func(txn *badger.Txn) error { item, err := txn.Get([]byte(nameHashKey)) if err != nil { return err } val, err := item.ValueCopy(nil) if err != nil { return err } ret = string(val) return nil }) - if err != nil { - return "", err - } - return ret, nil + return ret, err }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/indexer/handshake.go(2 hunks)internal/state/state.go(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
internal/indexer/handshake.go (2)
internal/handshake/covenant.go (8)
OpenCovenant(204-207)OpenCovenant(209-209)ClaimCovenant(161-168)ClaimCovenant(170-170)RegisterCovenant(327-332)RegisterCovenant(334-334)UpdateCovenant(366-370)UpdateCovenant(372-372)internal/state/state.go (1)
GetState(370-372)
⏰ 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: Analyze (go)
🔇 Additional comments (3)
internal/state/state.go (2)
11-11: LGTM!The addition of the
crypto/sha3import and the new key prefixes is well-structured and supports the handshake name tracking feature.Also applies to: 31-33
230-231: LGTM!The refactoring to use named constants (
recordKeyPrefix,domainKeyPrefix) instead of hardcoded strings improves maintainability and reduces the risk of typos in key construction.Also applies to: 257-257, 300-301
internal/indexer/handshake.go (1)
16-16: LGTM!The integration with the state layer to persist handshake names during OpenCovenant and ClaimCovenant processing is correct. Error handling appropriately propagates failures.
Also applies to: 116-123
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="internal/indexer/handshake.go">
<violation number="1" location="internal/indexer/handshake.go:125">
P1: Register/Update now abort handshake syncing whenever the name cache misses. Because the cache is only filled when future Open/Claim covenants run, every pre-existing domain causes `GetHandshakeNameByHash` to return `badger.ErrKeyNotFound`, which you bubble up and the sync loop stops processing blocks. Handle missing names as a non-fatal condition (log the hash and continue) and only return unexpected state errors.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| } | ||
| case *handshake.RegisterCovenant: | ||
| slog.Debug("Handshake domain registration", "resdata", c.ResourceData) | ||
| name, err := state.GetState().GetHandshakeNameByHash(c.NameHash) |
There was a problem hiding this comment.
P1: Register/Update now abort handshake syncing whenever the name cache misses. Because the cache is only filled when future Open/Claim covenants run, every pre-existing domain causes GetHandshakeNameByHash to return badger.ErrKeyNotFound, which you bubble up and the sync loop stops processing blocks. Handle missing names as a non-fatal condition (log the hash and continue) and only return unexpected state errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/indexer/handshake.go, line 125:
<comment>Register/Update now abort handshake syncing whenever the name cache misses. Because the cache is only filled when future Open/Claim covenants run, every pre-existing domain causes `GetHandshakeNameByHash` to return `badger.ErrKeyNotFound`, which you bubble up and the sync loop stops processing blocks. Handle missing names as a non-fatal condition (log the hash and continue) and only return unexpected state errors.</comment>
<file context>
@@ -112,10 +113,26 @@ func (i *Indexer) handshakeHandleSync(block *handshake.Block) error {
+ }
case *handshake.RegisterCovenant:
- slog.Debug("Handshake domain registration", "resdata", c.ResourceData)
+ name, err := state.GetState().GetHandshakeNameByHash(c.NameHash)
+ if err != nil {
+ return err
</file context>
Summary by cubic
Track Handshake name hashes and map them to raw names in state, so the indexer can resolve and log domain names during register/update events.
New Features
Refactors
Written for commit ed93565. Summary will update automatically on new commits.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.