feat(core): add assertDaoOutputLimit for NervosDAO 64-output guard#359
feat(core): add assertDaoOutputLimit for NervosDAO 64-output guard#359phroi wants to merge 2 commits intockb-devrel:masterfrom
Conversation
NervosDAO transactions silently fail on-chain when they exceed 64 outputs. This adds a proactive check: - `assertDaoOutputLimit(tx, client)` — throws `ErrorNervosDaoOutputLimit` when a DAO transaction has >64 outputs - Auto-called at the end of `completeFee` so callers get early failure instead of a rejected on-chain submission - Only fires when outputs > 64 AND at least one input/output carries the DAO type script — zero overhead for non-DAO transactions
🦋 Changeset detectedLatest commit: 8e63e3a The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for liveccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for appccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for apiccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary of ChangesHello @phroi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a crucial early warning system for NervosDAO transactions. It addresses the issue of transactions silently failing on-chain when they exceed the 64-output limit, which previously led to wasted fees and user frustration. By implementing a new validation function and integrating it into the transaction completion process, the system now proactively identifies and flags such invalid transactions, significantly improving the user experience and reliability of DAO operations. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces a crucial check for NervosDAO transactions to prevent silent failures due to exceeding the 64-output limit. This is a significant improvement for user experience and transaction reliability. The implementation correctly integrates the check into the completeFee method and provides a clear error message. The ErrorNervosDaoOutputLimit class is well-defined. The changes are well-documented and appear to be robust.
| // The fee has been paid | ||
| if (leastFee === changedFee) { | ||
| this.copy(tx); | ||
| await assertDaoOutputLimit(this, from.client); |
There was a problem hiding this comment.
The assertDaoOutputLimit function is also called here, after the transaction has been copied and the fee has been paid. This ensures that even if the transaction structure changes during the fee calculation, the DAO output limit is re-validated before returning, catching any potential issues introduced by the change function.
There was a problem hiding this comment.
Correct, the dual check is intentional: the change callback can alter outputs
✅ Deploy Preview for docsccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
phroi
force-pushed
the
feat/dao-output-limit
branch
from
395860e to
8e63e3a
Compare
|
@Hanssen0 @ashuralyk all green, feel free to review 🤗 Phroi %28 |
1 participant
Why
NervosDAO transactions silently fail on-chain when they exceed 64 outputs. There is no early warning — the transaction submits fine but gets rejected by the DAO script during verification, wasting fees and confusing users.
Changes
assertDaoOutputLimit(tx, client)inpackages/core/src/ckb/transaction.ts: throwsErrorNervosDaoOutputLimitwhen a DAO transaction has >64 outputsErrorNervosDaoOutputLimiterror class intransactionErrors.tscompleteFee(both return paths) for early failure