Skip to content

Conversation

@snawaz
Copy link
Contributor

@snawaz snawaz commented Oct 15, 2025

Issue

We encountered TooManySeeds (Custom Error: 14 or 0xe):

The older delegate had used Vec for seeds, but the new implementation uses fixed-size arrays to avoid heap allocation. These fixed-size arrays allowed upto 4 seeds. But the above failing instruction passes 5 seeds:

  • ["token_escrow", followed by 3 seeds (32 bytes each), then an 8-byte zero seed]

Instruction data (after decoding):

[0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 255, 255, 5, 0, 0, 0, 12, 0, 0, 0, 116, 111, 
107, 101, 110, 95, 101, 115, 99, 114, 111, 119, 32, 0, 0, 0, 224, 44, 225, 167, 
90, 132, 166, 170, 152, 1, 226, 3, 185, 117, 139, 9, 56, 129, 121, 8, 149, 189, 
81, 139, 183, 63, 79, 36, 246, 229, 194, 175, 32, 0, 0, 0, 241, 3, 142, 54, 165, 
6, 236, 130, 120, 166, 44, 190, 90, 71, 127, 231, 65, 218, 208, 252, 118, 31, 243, 
145, 160, 22, 98, 89, 109, 143, 80, 86, 32, 0, 0, 0, 198, 250, 122, 243, 190, 219,
173, 58, 61, 101, 243, 106, 171, 201, 116, 49, 177, 187, 228, 194, 210, 246, 224,
228, 124, 166, 2, 3, 69, 47, 93, 97, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]

Fix

This PR increases the limit on the number of seeds from 4 to 8.

Cost Comparison

If we use Vec and ready to pay some extra CUs, then:

  • Using fix-sized arrays, delegate consumes 18139 CU.
  • Using Vec, delegate consumes 18268 CU.
  • So the difference is 18268 - 18139 = 129 CU.

If we continue with fixed-size array approach, we can define a macro to reduce the verbosity in the code and can use it wherever we have such opportunity, something like:

let seeds_to_validate: &[&u8]] = as_slices!(args.seeds, limit: 8);

Summary by CodeRabbit

  • New Features
    • Expanded seed validation to support 5–8 seeds in address-related flows. You can now provide up to eight seeds where applicable, enabling more flexible and complex configurations. Behavior for other counts is unchanged, and invalid counts still return an error. No action required; existing workflows continue to function as before.

Copy link
Contributor Author

snawaz commented Oct 15, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@magicblock-labs magicblock-labs deleted a comment from coderabbitai bot Oct 15, 2025
@snawaz snawaz requested a review from GabrielePicco October 15, 2025 09:20
@snawaz snawaz marked this pull request as ready for review October 15, 2025 09:20
@snawaz snawaz changed the title hotfix: Increase the limit of seeds from 4 to 8 for escrow accounts fix: Increase the limit of seeds from 4 to 8 for escrow accounts Oct 15, 2025
@magicblock-labs magicblock-labs deleted a comment from coderabbitai bot Oct 15, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 15, 2025

\n\n## Walkthrough\nExpanded seed-count handling in src/processor/fast/delegate.rs: the seeds_to_validate match now supports 5–8 seeds by constructing matching seed arrays. Behavior for other counts and subsequent derived_pda computation/validation remains unchanged.\n\n## Changes\n| Cohort / File(s) | Summary of changes |\n| --- | --- |\n| Seed validation logic
src/processor/fast/delegate.rs | Extend seeds_to_validate match to support 5–8 seeds; construct corresponding seed arrays. Default TooManySeeds unchanged. No changes to derived_pda computation or later validation flow. |\n\n## Sequence Diagram(s)\nmermaid\nsequenceDiagram\n autonumber\n participant Caller\n participant Delegate as delegate.rs\n participant PDA as PDA Deriver\n participant Validator\n\n Caller->>Delegate: validate(seeds_to_validate)\n Note over Delegate: Match on seeds_to_validate length<br/>(1–4 existing, 5–8 newly handled)\n Delegate->>Delegate: Build seed array (1–8)\n Delegate->>PDA: derive_pda(seed array)\n PDA-->>Delegate: derived_pda\n Delegate->>Validator: validate_with_pda(derived_pda)\n Validator-->>Delegate: result\n Delegate-->>Caller: result\n alt >8 seeds\n Delegate-->>Caller: Error: TooManySeeds\n end\n\n\n## Estimated code review effort\n🎯 2 (Simple) | ⏱️ ~10 minutes\n\n

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The provided description includes an “Issue” and “Fix” section but does not follow the repository’s required template which begins with a status/type table and uses headings like “Problem” and “Solution.” It also omits the top-level note and sections for “Before & After Screenshots,” “Other changes,” and “Deploy Notes.” While the narrative explains the root cause and fix clearly, it does not adhere to the structured template expected for consistency and complete context. Because the template ensures maintainers have all necessary information in a familiar format, the description needs updating to align with it. Please revise the pull request description to include the required status/type table with PR status, type, core change flag, and issue link, rename “Issue” to “Problem” and “Fix” to “Solution,” and add the “Before & After Screenshots,” “Other changes,” and “Deploy Notes” sections or note if they are not applicable.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely describes the primary change by stating that the seed limit is increased from four to eight for escrow accounts, which directly matches the core update in the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch snawaz/hotfix

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

Copy link
Contributor

@GabrielePicco GabrielePicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

We should support up to 16 seeds to cover all cases, but we can merge as it is to cover already most use-cases.

@snawaz
Copy link
Contributor Author

snawaz commented Oct 15, 2025

LGTM!

We should support up to 16 seeds to cover all cases, but we can merge as it is to cover already most use-cases.

Yes. Created an issue: #108

@snawaz snawaz merged commit e0975c6 into main Oct 15, 2025
5 checks passed
@snawaz snawaz deleted the snawaz/hotfix branch October 15, 2025 12:17
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.

3 participants