Align sequence locks and unknown covenant handling with hsd#53
Conversation
Signed-off-by: Chris Gianelloni <wolf31o2@blinklabs.io>
📝 WalkthroughWalkthroughSequence-lock semantics no longer depend on transaction version, allowing version-zero transactions in chain calculation and Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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: 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 `@blockchain/names.go`:
- Around line 24-26: Update maxCovenantSize in names.go to derive from hsd’s
current consensus formula, using the existing maxResourceSize value and removing
the stale extra fields. Address the duplicated maxCovenantItems definition
between names.go and wire/covenant.go by centralizing the constant or adding a
clear cross-reference so both packages remain synchronized.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cbea625f-fd63-4563-8f34-ad4a2fc37f94
📒 Files selected for processing (8)
blockchain/chain.goblockchain/chain_test.goblockchain/names.goblockchain/names_test.gotxscript/engine_test.gotxscript/opcode.gowire/covenant.gowire/covenant_test.go
💤 Files with no reviewable changes (1)
- txscript/opcode.go
| maxCovenantItems = 1000 | ||
| maxCovenantSize = 1 + 32 + 1 + 4 + 2 + maxResourceSize + 1 + 32 | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for maxCovenantSize or maxResourceSize definitions and usage
rg -nP 'maxCovenantSize|maxResourceSize' blockchain/names.go
rg -nP 'maxCovenantItems' blockchain/ wire/Repository: blinklabs-io/handshake-node
Length of output: 1251
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant logic in the local repository.
sed -n '1,120p' blockchain/names.go
printf '\n---\n'
sed -n '860,910p' blockchain/names.go
printf '\n---\n'
sed -n '1,220p' wire/covenant.goRepository: blinklabs-io/handshake-node
Length of output: 9338
🌐 Web query:
hsd covenant size limit maxCovenantSize maxResourceSize covenantItemsSize consensus constant
💡 Result:
In the hsd implementation of the Handshake protocol, covenant size limits are governed by constants defined in lib/covenants/rules.js [1][2]. The relevant consensus-related constraints are: MAX_RESOURCE_SIZE: This is set to 512 bytes [1][2]. It defines the maximum allowed size for record data within a covenant (specifically used for items[2] in a covenant) [1][2][3]. MAX_COVENANT_SIZE: This represents the maximum total serialized size of a covenant [1][2]. It is calculated as a constant expression: 2 + MAX_RESOURCE_SIZE + 1 + 32 [1][2]. Given MAX_RESOURCE_SIZE is 512, this results in a maximum size of 547 bytes [1][2]. These rules are enforced during transaction validation to ensure that covenant data, such as DNS resource records, remains within the defined protocol constraints [1][2][3]. While consensus.js manages network-wide constants like block size (1,000,000 bytes) [4][5], the covenant-specific limits are handled within the covenants module to maintain naming system integrity [6][7].
Citations:
- 1: https://coveralls.io/builds/76715009/source?filename=lib%2Fcovenants%2Frules.js
- 2: https://coveralls.io/builds/75438713/source?filename=lib%2Fcovenants%2Frules.js
- 3: https://deepwiki.com/handshake-org/hsd/9.1-covenant-types
- 4: https://hsd-dev.org/hsd/protocol_consensus.js.html
- 5: https://hsd-dev.org/hsd/module-protocol_consensus.html
- 6: https://hsd-dev.org/hsd/hsd.js.html
- 7: https://hsd-dev.org/protocol/summary.html
🌐 Web query:
hsd lib/covenants/rules.js MAX_COVENANT_SIZE MAX_RESOURCE_SIZE formula unknown covenant items size
💡 Result:
In the hsd library, the file lib/covenants/rules.js manages protocol limits for Handshake name covenants [1][2]. The constants MAX_COVENANT_SIZE and MAX_RESOURCE_SIZE are defined to enforce consensus rules regarding the size of covenants and their embedded resource data [1][3]. MAX_RESOURCE_SIZE is the limit on the size of record data within a covenant. Historically, this limit was 512 bytes [1][3]. Following a protocol hard fork (as noted in recent updates), this limit has been increased to 8,192 bytes to accommodate richer zone files [3][4]. The calculation for MAX_COVENANT_SIZE is defined in the source code as: rules.MAX_COVENANT_SIZE = (0 + 1 + 32 + 2 + rules.MAX_RESOURCE_SIZE + 1 + 32); In this formula, the components represent: - 1: A byte for the covenant type. - 32: A 32-byte hash (typically the name hash). - 2: An overhead allowance (or specific protocol field allocation). - rules.MAX_RESOURCE_SIZE: The maximum size permitted for the resource record data [1][5]. - 1 + 32: Additional structural or metadata padding used in the covenant serialization [1][6]. Essentially, the total covenant size limit is derived from the sum of these fixed-size fields plus the current maximum allowed resource size [1]. Enforcement of these limits occurs during block validation, where the system checks both the length of specific items (like record data) and the total size of the covenant object using these defined constants [1][3].
Citations:
- 1: https://coveralls.io/builds/76715027/source?filename=lib%2Fcovenants%2Frules.js
- 2: https://deepwiki.com/handshake-org/hsd/9-name-covenant-system
- 3: https://github.com/handshake-org/hsd/pull/952/files
- 4: Handshake Hard Fork - Arbitrary DNS Records, Supply Reallocation, and Tree Interval handshake-org/hsd#952
- 5: https://coveralls.io/builds/75874803/source?filename=lib%2Fcovenants%2Frules.js
- 6: https://coveralls.io/builds/75438713/source?filename=lib%2Fcovenants%2Frules.js
Align maxCovenantSize with hsd's current consensus bound. hsd now derives MAX_COVENANT_SIZE from 1 + 32 + 2 + MAX_RESOURCE_SIZE + 1 + 32, and MAX_RESOURCE_SIZE is 8192 upstream, so the local 1 + 32 + 1 + 4 + 2 + maxResourceSize + 1 + 32 bound is stale and can diverge from consensus for unknown covenants. Also, maxCovenantItems = 1000 is duplicated in wire/covenant.go; add a cross-reference or centralize it to keep the two packages in sync.
🤖 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 `@blockchain/names.go` around lines 24 - 26, Update maxCovenantSize in names.go
to derive from hsd’s current consensus formula, using the existing
maxResourceSize value and removing the stale extra fields. Address the
duplicated maxCovenantItems definition between names.go and wire/covenant.go by
centralizing the constant or adding a clear cross-reference so both packages
remain synchronized.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 8 files
You're on the cubic free plan with 11 free PR reviews remaining this month. Upgrade for unlimited reviews.
Re-trigger cubic
Signed-off-by: Chris Gianelloni <wolf31o2@blinklabs.io>
1 participant
Summary
Validation
go test ./wire ./txscript ./blockchaingit diff --checkReview notes
Consensus-critical. This removes inherited Bitcoin tx-version gates and avoids rejecting reserved future covenant types at wire/sanity layers.
Summary by cubic
Aligns consensus with hsd by applying sequence locks/OP_CHECKSEQUENCEVERIFY to all non-coinbase transactions and permitting unknown covenants under hsd stack and serialized-size bounds. Fixes unknown covenant size checks to match hsd’s MAX_COVENANT_SIZE.
New Features
OP_CHECKSEQUENCEVERIFYapply to all tx versions inblockchain/txscript(removed version ≥2 gate); added version-0 engine test.wire(no type cap;IsKnown/IsUnknownhelpers); bounds enforced:MaxCovenantItems=1000, per-item ≤585 bytes, and hsd-style max serialized size inblockchain; allow unknown→NONE, reject unknown→known name; tests added.Bug Fixes
Written for commit f3f7ec6. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
OP_CHECKSEQUENCEVERIFYnow work correctly for transactions of all versions.