feat(sdks): sealevel fee support svm sdk [IGNORE FOR NOW]#8647
feat(sdks): sealevel fee support svm sdk [IGNORE FOR NOW]#8647
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8647 +/- ##
==========================================
- Coverage 79.33% 73.02% -6.32%
==========================================
Files 143 7 -136
Lines 4278 708 -3570
Branches 436 32 -404
==========================================
- Hits 3394 517 -2877
+ Misses 855 190 -665
+ Partials 29 1 -28
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| } | ||
|
|
||
| function decodeFeeDataStrategy(c: ByteCursor): SvmFeeDataStrategy { | ||
| const kind = c.readU8() as FeeStrategyKind; |
There was a problem hiding this comment.
🔴 Unchecked as FeeStrategyKind cast on raw byte from on-chain data
c.readU8() returns number, but the cast as FeeStrategyKind narrows it to 0 | 1 | 2 without any validation. If the on-chain account contains a strategy kind value outside that range (e.g., a newer program version adds kind 3), the cast silently accepts it, violating the SvmFeeDataStrategy discriminated union type. Downstream pattern-match code in fee-strategy-utils.ts:35-46 uses an exhaustive never check that would throw at runtime, but the cast hides the real error origin. Per REVIEW.md mandatory type cast audit: every as must be flagged, and the only acceptable cast requires a // CAST: comment explaining why it's unavoidable.
| const kind = c.readU8() as FeeStrategyKind; | |
| const kind = c.readU8(); | |
| if (kind !== 0 && kind !== 1 && kind !== 2) { | |
| throw new Error(`Unknown FeeDataStrategy kind: ${kind}`); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| beneficiary, | ||
| maxFee: maxFee.toString(), | ||
| halfAmount: halfAmount.toString(), | ||
| } as C, |
There was a problem hiding this comment.
🔴 Unsafe as C type cast bypasses generic constraint in leaf fee reader
The object literal is cast as C where C extends LeafFeeConfig. The constructed object only has { type, owner, beneficiary, maxFee, halfAmount }, but LeafFeeConfig is a union that includes OffchainQuotedLinearFeeConfig which additionally requires quoteSigners: string[]. While current subclasses only instantiate C with types that match this shape, the cast circumvents TypeScript's ability to catch future misuse if a new leaf type with extra required fields is added. Per REVIEW.md mandatory type cast audit: "as X — flag it. The fix is almost always to fix the function signature, add a type guard, or restructure the code." No // CAST: comment is present.
Prompt for agents
In typescript/svm-sdk/src/fee/leaf-fee.ts line 91, the object literal is cast `as C`. The REVIEW.md mandates no `as` casts without a `// CAST:` comment. The fix is to either: (1) add a `// CAST: C is constrained to Linear/Regressive/Progressive which all share this exact shape; OffchainQuotedLinear has its own reader` comment if the cast is truly unavoidable, or (2) restructure to avoid the cast entirely, e.g. by having each concrete reader build its own config object and returning it without a cast.
Was this helpful? React with 👍 or 👎 to provide feedback.
| routerToBytes(router!), | ||
| ), | ||
| ], | ||
| annotation: `Remove CC route for domain ${domainStr} router ${router!.slice(0, 10)}...`, |
There was a problem hiding this comment.
🟡 Non-null assertions router! without provable non-nullness on preceding line
On lines 302 and 305, router! is used after const [domainStr, router] = key.split(':') on line 293. TypeScript types the second destructured element as string | undefined since split returns string[] and destructuring beyond the array length yields undefined. While the key format ${domain}:${router} guarantees a colon exists, the non-null assertion is not provably safe from the immediately preceding line — it depends on reasoning about how the key was constructed several lines earlier. Per REVIEW.md: "! (non-null assertion) — flag unless the value is provably non-null on the preceding line."
| routerToBytes(router!), | |
| ), | |
| ], | |
| annotation: `Remove CC route for domain ${domainStr} router ${router!.slice(0, 10)}...`, | |
| routerToBytes(router ?? ''), | |
| ), | |
| ], | |
| annotation: `Remove CC route for domain ${domainStr} router ${(router ?? '').slice(0, 10)}...`, |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const primaryPath = join(PROGRAMS_DIR, filename); | ||
| const fallbackPath = join(LOCAL_SO_DIR, filename); | ||
| let bytes; | ||
| try { | ||
| const bytes = readFileSync(path); | ||
| programBytes[key] = Array.from(bytes); | ||
| console.log(` ✅ ${key}: ${bytes.length.toLocaleString()} bytes`); | ||
| bytes = readFileSync(primaryPath); | ||
| } catch { | ||
| console.error(` ❌ ${key}: required .so file not found at ${path}`); | ||
| process.exit(1); | ||
| try { | ||
| bytes = readFileSync(fallbackPath); | ||
| } catch { | ||
| console.error(` ❌ ${key}: .so not found at ${primaryPath} or ${fallbackPath}`); | ||
| process.exit(1); | ||
| } | ||
| } |
There was a problem hiding this comment.
🚩 Fee program binary added to program-bytes generator with fallback path mechanism
In scripts/generate-program-bytes.mjs, the new tokenFee entry was added at line 28, and lines 36-48 introduce a fallback mechanism that checks LOCAL_SO_DIR (so-binaries/) when the primary path (rust/sealevel/target/deploy/) doesn't have the binary. This is a deviation from the existing pattern where all programs came from the same build directory. The fallback could mask build issues if a stale local binary is used instead of a freshly compiled one. The check-program-bytes-hash.mjs script should catch staleness via the source hash, but only if it covers the fee program sources.
Was this helpful? React with 👍 or 👎 to provide feedback.
…el-token-fee-support Copies the compiled fee program .so into so-binaries/ and updates the generation script to include tokenFee with a fallback to the local directory. Drop this commit once the fee program lands on main and is built normally.
Added deriveFeeAccountPda, deriveRouteDomainPda, deriveCrossCollateralRoutePda, deriveTransientQuotePda, and deriveStandingQuotePda to the PDA utilities module.
Added SvmDeployedFee, SvmFeeWriterConfig, DEFAULT_FEE_SALT, deriveFeeSalt, FeeStrategyKind/FeeDataKind enums, and H160 signer address conversion utilities.
Added FeeParams, FeeDataStrategy, FeeData, LeafFeeConfig, RoutingFeeConfig, CrossCollateralRoutingFeeConfig, RouteKey types with Borsh encoding for instruction data serialization. Includes BTreeSet<H160> and discriminator constants.
Added ByteCursor.readI64LE() and i64le() encoder for signed 64-bit integers. Added Borsh decoders for FeeAccount, RouteDomain, CrossCollateralRoute, TransientQuote, and StandingQuotePda account types. Fixed RoutingFeeConfig and CrossCollateralRoutingFeeConfig wildcard_signers encoding to match on-chain BTreeSet (non-optional) layout.
Added InitFee, UpdateFeeParams, SetBeneficiary, and TransferOwnership instruction builders with FeeInstructionKind enum covering all 18 instructions.
Added fetchFeeAccount for reading on-chain fee state and detectSvmFeeType for mapping on-chain FeeData variants to provider-sdk FeeType values.
SvmLinearFeeReader reads Leaf(Linear) fee accounts and returns LinearFeeConfig. SvmLinearFeeWriter creates and updates linear fee accounts with param, beneficiary, and ownership diffing using existing address comparison utilities.
Tests create, read, no-op update, fee param update, and beneficiary update. Each test deploys a fresh program instance for full isolation.
…iter Extracted SvmLeafFeeReader/Writer abstract base with abstract feeType and strategyKind properties. Slimmed down linear-fee.ts to thin subclass. Added SvmRegressiveFeeReader/Writer following the same pattern.
…tests Extracted defineLeafFeeTests shared suite for leaf fee types. Refactored linear test to use it. Added regressive fee E2E tests using the same suite.
Added SvmProgressiveFeeReader/Writer extending the leaf fee base. E2E tests validate create, read, no-op update, param update, and beneficiary update.
…uilders Handles all three route variants: Leaf (fee_account writable, no route PDA), Routing (route PDA writable), and CC (cc_route PDA writable).
Reads Leaf(Linear) with signers and maps to OffchainQuotedLinearFeeConfig. Writer creates with signers via InitFee, update diffs params, beneficiary, signer set (add/remove), and ownership.
Reuses shared leaf test suite for common cases (create/read, params, beneficiary). Adds signer-specific tests: read signers after create, add signer, remove signer. Widened LeafFeeConfig union to include OffchainQuotedLinearFeeConfig.
… instruction builders SetRoute creates/updates per-domain RouteDomain PDAs for Routing mode. RemoveRoute closes a RouteDomain PDA returning rent to owner. SetWildcardQuoteSigners sets the wildcard signer set on the fee account.
Fetches and decodes a RouteDomain PDA for a given fee account and destination domain.
Reader uses FeeReadContext to discover which domain route PDAs to query (route PDAs are not enumerable on-chain). Maps on-chain RouteDomain data to provider-sdk FeeStrategy including offchainQuotedLinear detection. Writer deploys Routing fee with per-domain routes and computed wildcard signers. Update diffs routes (add/update/remove), wildcard signers, beneficiary, and ownership.
Tests create/read with multiple routes, offchainQuotedLinear route with signers, add new route, update existing route params, change route strategy type, and remove route.
…oute instruction builders SetCrossCollateralRoute creates/updates per-(destination, targetRouter) CC route PDAs. RemoveCrossCollateralRoute closes a CC route PDA returning rent to owner.
Fetches and decodes a CrossCollateralRoute PDA for a given fee account, destination domain, and target router address.
…ract shared strategy utils Extracted routeDataToFeeStrategy, feeStrategyToOnChain, and computeWildcardSignersFromStrategies into fee-strategy-utils.ts, shared by both routing and CC routing implementations. CC routing reader uses FeeReadContext (domain, router) pairs to discover CC route PDAs. Writer deploys CC routing fee with per-(domain, router) routes and computed wildcard signers. Update diffs routes, wildcard signers, beneficiary, and ownership.
Tests create/read with multiple (domain, router) pairs, add new pair, update existing route params, change strategy type, and remove route. All update tests assert exact expected transaction counts.
Added SetMinIssuedAt, SubmitQuote (transient and standing variants), CloseTransientQuote, PruneExpiredQuotes, GetQuoteAccountMetas, GetSubmitQuoteAccountMetas, and QuoteFee instruction builders. Includes SvmSignedQuoteData type and Borsh encoder for the quote payload.
Implements IRawFeeArtifactManager with readFee (auto-detects fee type), createReader, and createWriter factory methods for all 6 fee types.
…add exports SvmProtocolProvider.createFeeArtifactManager() now returns a real SvmFeeArtifactManager instead of null. Added all fee reader/writer, type, PDA, and artifact manager exports to the public API following the existing export pattern.
Binary search + incremental growth tests that find the maximum: - InitFee (Leaf) signers: 41 (tx size limit) - SetRoute signers: 42 (tx size limit) - Incremental AddQuoteSigner on Leaf: 409 (CU/heap limit) - SetWildcardQuoteSigners: 47 (tx size limit) Tx size bottleneck: ~1232 byte legacy tx limit, each H160 signer = 20 bytes. CU/heap bottleneck: BTreeSet deserialization at ~400 entries exhausts compute. Standing quote limits require secp256k1 signing infrastructure (noted as TODO).
Binary search + incremental growth tests that find the maximum: TX SIZE LIMITS (~1232 byte legacy tx payload, each H160 = 20 bytes): - InitFee (Leaf) max signers: 41 - SetRoute max signers: 42 - SetWildcardQuoteSigners max signers: 47 CU / HEAP LIMITS (BTreeSet/BTreeMap deser on 32KB heap): - AddQuoteSigner incremental on Leaf: 409 - Standing quotes per domain PDA: 113 (Leaf and CC identical) Added @noble/curves as devDependency for secp256k1 quote signing in tests.
- High: fix empty signer sets misread as no signers. Use isNullish() to distinguish None from Some(empty) — on-chain Some([]) means offchain quoting enabled with zero signers, not on-chain-only mode. - Medium: fix ownerless fee deployment. Use isZeroishAddress() instead of truthiness to detect zero-address sentinel in all create paths (leaf, offchain-quoted, routing, CC routing). - Medium: fix unsafe signer/router hex parsing. Use addressBytes() + ensureLength() from codecs/binary.ts for validated conversion in signerToH160() and routerToBytes(). - Low: fix BTreeSet codec to sort entries before encoding, matching Rust canonical BTreeSet serialization order. - Medium (documented): signers are orthogonal to strategy on-chain but provider-sdk only models offchainQuotedLinear. Added comments noting this limitation in detectSvmFeeType and routeDataToFeeStrategy.
… createFeeArtifactManager Made FeeReadContext a required parameter on ProtocolProvider.createFeeArtifactManager() so that the deploy-sdk factories pass the context from the warp config all the way through to protocol-specific artifact managers. This fixes the high-severity issue where SVM routing/CC fee updates saw empty routes because context was lost. The deploy-sdk FeeReader/FeeWriter factories now pass context to the protocol provider. All protocol SDK implementations updated to accept the new parameter.
…uotedLinear type Validates that removing the last signer results in Some(empty set) on-chain, not None, and the fee type reads back as offchainQuotedLinear (not linear).
Added fee-linear, fee-regressive, fee-progressive, fee-offchain-quoted-linear, fee-routing, and fee-cross-collateral-routing to the SVM SDK e2e test matrix. Stress tests excluded (exploratory, long-running).
- Replace unsafe `as FeeStrategyKind` cast with `isFeeStrategyKind()` type guard that validates at runtime and narrows the type via `value is`. - Add CAST comment on `as C` in leaf-fee reader explaining why OffchainQuotedLinearFeeConfig never flows through this path. - Replace `router!` non-null assertions with `assert(router)` in CC routing update diff. - Replace `!` non-null assertions in BTreeSet sort comparator with nullish coalescing. - Replace `expected.routes[domain]!` with Object.entries iteration.
xeno097
force-pushed
the
xeno/sealevel-fee-support-svm-sdk
branch
from
79ebd4c to
22a40cd
Compare
Node Services Docker Image Built Successfully
Full image paths |
1 participant
Description
Implements Phase 2 of the SVM fee system: full SDK support for deploying and managing fee programs on Solana/SVM chains via the
IRawFeeArtifactManagerinterface.Requires #8642 (SVM fee program implementation) to be merged first. The fee program binary is temporarily injected via a droppable commit (
0ae46733a6) that should beremoved once the program lands on main.
What's included:
(linear/regressive/progressive) share an abstract base to avoid duplication.
SetWildcardQuoteSigners, SetMinIssuedAt, RemoveRoute, RemoveCrossCollateralRoute) and runtime instructions (QuoteFee, SubmitQuote transient/standing, CloseTransientQuote, PruneExpiredQuotes, GetQuoteAccountMetas,
GetSubmitQuoteAccountMetas).
IRawFeeArtifactManager, wired intoSvmProtocolProvider.createFeeArtifactManager().Drive-by changes
ProtocolProvider.createFeeArtifactManager()now requires aFeeReadContextparameter to ensure routed fee types receive domain/router context for non-enumerable route PDA discovery. All protocol SDK implementations(tron, radix, cosmos, aleo, starknet) updated to accept the new parameter.
ByteCursor.readI64LE()andi64le()encoder for signed 64-bit integer support.@noble/curvesas a devDependency in svm-sdk for secp256k1 quote signing in stress tests.Related issues
Backward compatibility
No —
ProtocolProvider.createFeeArtifactManager()has a new requiredFeeReadContextparameter. This is a major version bump for@hyperlane-xyz/provider-sdkand@hyperlane-xyz/deploy-sdk. All downstream protocol SDK consumers needto pass the context. The deploy-sdk factories (
createFeeReader,createFeeWriter) already had context and now thread it through.Testing
E2E tests (32 tests across 6 suites, all in CI):