Skip to content

Fix/zcash zip317 nu6.1 fees#1672

Merged
Thorian1te merged 3 commits intomasterfrom
fix/zcash-zip317-nu6.1-fees
Mar 31, 2026
Merged

Fix/zcash zip317 nu6.1 fees#1672
Thorian1te merged 3 commits intomasterfrom
fix/zcash-zip317-nu6.1-fees

Conversation

@Thorian1te
Copy link
Copy Markdown
Collaborator

@Thorian1te Thorian1te commented Mar 31, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Resolved memo fee double-counting issue in Zcash transactions, eliminating unexpected fee inflation when transaction memos are included
    • Unified fee calculation methodology across all transaction creation pathways to provide consistent, accurate, and predictable transaction fees

Thorian1te and others added 3 commits March 31, 2026 11:42
NU6.1 (activated Nov 24 2025 at block 3146400) changed fee rules:
- MARGINAL_FEE: 5000 → 10000 zatoshi (doubled)
- GRACE_ACTIONS: 2 → 0 (removed, all actions must be paid)
- Logical actions: inCount + outCount → max(inCount, outCount)

Without this fix, transactions are rejected with:
"-26: 66: tx unpaid action limit exceeded"

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
getFee is now the single source of truth for memo output slot counting.
Callers pass only non-memo outputs; getFee adds memo slots internally.
Fixes double-counting in buildTx/prepareMaxTx and bypass in buildMaxTx.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1a93e6fb-daf1-44a2-976f-e4f4a7656906

📥 Commits

Reviewing files that changed from the base of the PR and between 06826b5 and 6df2034.

📒 Files selected for processing (4)
  • .changeset/fix-zcash-memo-fees.md
  • packages/xchain-zcash/__tests__/client.test.ts
  • packages/xchain-zcash/src/client.ts
  • packages/zcash-js/src/builder.ts

📝 Walkthrough

Walkthrough

This PR fixes memo fee double-counting in Zcash by standardizing how memo output slots are calculated. It increases the marginal fee constant, refactors fee calculation logic to use Math.max(inCount, outCount) instead of summing both with a grace floor, and updates both the builder and client to use fixed output counts while delegating memo slot accounting to the getFee function.

Changes

Cohort / File(s) Summary
Release Notes
.changeset/fix-zcash-memo-fees.md
Added changeset documenting patch release fixing memo fee double-counting and standardizing memo output slot calculation in Zcash packages.
Zcash Client
packages/xchain-zcash/__tests__/client.test.ts, packages/xchain-zcash/src/client.ts
Updated fee test expectations to reflect new calculations; modified prepareMaxTx to use fixed output count (1), delegating memo slot accounting to getFee.
Zcash Builder
packages/zcash-js/src/builder.ts
Increased MARGINAL_FEE from 5000 to 10000; refactored calculateFee to use Math.max(inCount, outCount) instead of input+output sum; standardized buildTx and buildMaxTx to use fixed output counts with getFee handling memo slots.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • underthesun49
  • Naq302
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/zcash-zip317-nu6.1-fees

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Thorian1te Thorian1te merged commit 5b56da3 into master Mar 31, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants