Sub vaults#339
Open
dfkadyr wants to merge 24 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces functionality to retrieve and manage sub-vaults within a meta vault structure, allowing users to query information about vaults that are allocated within a parent meta vault.
Changes:
- Added
getSubVaultsmethod to retrieve sub-vault information including APY, assets, and exit queue data - Extended the allocator GraphQL query to include APY field
- Added
isStateUpdateRequiredfunction to the DefaultVaultAbi contract
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/vault/requests/index.ts | Exports the new getSubVaults function and its input type |
| src/services/vault/requests/getSubVaults/index.ts | Implements the main logic for fetching and aggregating sub-vault data |
| src/services/vault/requests/getSubVaults/getSubVaults.md | Provides API documentation for the getSubVaults method |
| src/services/vault/index.ts | Adds the public getSubVaults method to the Vault service class |
| src/graphql/subgraph/vault/subVaultsQuery.graphql | Defines the GraphQL query for fetching sub-vaults |
| src/graphql/subgraph/vault/index.ts | Exports the SubVaults query types and fetch function |
| src/graphql/subgraph/allocator/allocatorsQuery.graphql | Adds apy field to the allocators query |
| src/contracts/vault/abis/DefaultVaultAbi.json | Adds isStateUpdateRequired function to the vault ABI |
| changelog/next-release.md | Documents the addition of the getSubVaults method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mike-diamond
previously approved these changes
Jan 21, 2026
| } | ||
| ] | ||
|
|
||
|
|
Cast0001
previously approved these changes
Jan 21, 2026
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 85 out of 89 changed files in this pull request and generated 11 comments.
Comments suppressed due to low confidence (3)
codegen.ts:10
codegen.tsnow defaultsnetworktoNetwork.Hoodiwith a TODO. This will change generated GraphQL/types for anyone running codegen withoutNETWORKset, which is risky for a published SDK. Consider reverting the default back toNetwork.Mainnetand using env vars (or a separate script/flag) for generating Hoodi/stage schemas.
// TODO change
let network: Network = Network.Hoodi
if (process.env.NETWORK === 'mainnet') {
network = Network.Mainnet
}
src/services/vault/transactions/createVault/helpers/validateCreateVaultArgs.ts:98
metaVault()validation logic is inverted: itreturns whentypeis a MetaVault, so the subsequent MetaVault-only restrictions are applied to non-MetaVault vaults instead (e.g., it would throw onisOwnMevEscrowfor normal vaults, and on Gnosis it could incorrectly block ERC20/private non-MetaVault vault creation). Flip the condition so these checks run only whentypeisMetaVault/PrivateMetaVault(and consider renamingisMainnetsince it includes Hoodi).
.graphqlconfig:18.graphqlconfigis updated to point at the Hoodistagesubgraph endpoint. This looks like a local/dev change and can cause confusion for contributors and CI tooling that relies on this config. Consider keeping.graphqlconfigpointed at the canonical/prod endpoint and overriding locally via environment or editor settings.
"Subgraph GraphQL": {
"url": "https://graphs.stakewise.io/hoodi/subgraphs/name/stakewise/stage",
"headers": {
"user-agent": "JS GraphQL"
},
"introspect": false
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| isSmoothingPool: boolean | ||
| tokenName: string | null | ||
| whitelistManager: string | ||
| whitelistManager: string |
Comment on lines
+10
to
+11
| // TODO change (+ change .graphqlconfig) | ||
| subgraph: 'https://graphs.stakewise.io/hoodi/subgraphs/name/stakewise/stage', |
Comment on lines
+54
to
+83
| const results = await Promise.all( | ||
| subVaults.map(async (subVaultId) => { | ||
| const vaultId = subVaultId.toLowerCase() | ||
|
|
||
| const [ allocatorData, vaultData, exitQueue ] = await Promise.all([ | ||
| graphql.subgraph.allocator.fetchAllocatorsQuery({ | ||
| url, | ||
| variables: { | ||
| address: metaVaultId, | ||
| vaultAddress: vaultId, | ||
| }, | ||
| modifyResult: (data) => { | ||
| const first = data?.allocators?.[0] | ||
|
|
||
| return { | ||
| assets: BigInt(first?.assets || 0), | ||
| apy: (Number(first?.apy) || 0).toFixed(2), | ||
| } | ||
| }, | ||
| }), | ||
|
|
||
| getVault({ ...commonParams, vaultAddress: vaultId }), | ||
|
|
||
| getExitQueuePositions({ | ||
| ...commonParams, | ||
| isClaimed: false, | ||
| vaultAddress: vaultId, | ||
| userAddress: metaVaultId, | ||
| }), | ||
| ]) |
Comment on lines
+16
to
+17
| | limit | `number` | **No** | Limits the number of sub vaults returned. Defaults to 100 | | ||
| | skip | `number` | **No** | Skips the specified number of sub vaults. Defaults to 0 | |
Comment on lines
+4
to
+13
| import { checkAccess } from '../../../vault/transactions/util' | ||
|
|
||
|
|
||
| const updateFeeRecipients = async (values: UpdateFeeRecipientsInput) => { | ||
| const updateFeeRecipients = checkAccess<UpdateFeeRecipientsInput>(async (values) => { | ||
| const multicallArgs = await commonLogic(values) | ||
|
|
||
| const result = await rewardSplitterMulticall<{ hash: string }>(multicallArgs) | ||
|
|
||
| return result.hash | ||
| } | ||
| }) |
| #### Add output field: | ||
| ```ts | ||
| type Output = { | ||
| canHarvest: boolen |
Comment on lines
4
to
+16
| @@ -12,7 +13,7 @@ const createRewardSplitter = async (values: CreateRewardSplitterInput) => { | |||
| ) | |||
|
|
|||
| return result.hash | |||
| } | |||
| }) | |||
Comment on lines
+23
to
27
| const harvestArgs = await getHarvestArgs(values) | ||
|
|
||
| const baseParams: BaseParams = [ userAddress, referrerAddress, overrides ] | ||
| const updateStateParams: UpdateStateParams = [ userAddress, referrerAddress, harvestParams, overrides ] | ||
| const updateStateParams: UpdateStateParams = [ userAddress, referrerAddress, harvestArgs as HarvestArgs, overrides ] | ||
|
|
Comment on lines
+22
to
+29
| const vaultFactory = getVaultFactory({ | ||
| ...values, | ||
| isErc20: Boolean(vaultToken), | ||
| vaultType: type, | ||
| }) as typeof isMetaVault extends true | ||
| ? StakeWise.ABI.MetaVaultFactory | ||
| : StakeWise.ABI.VaultFactory | ||
|
|
Comment on lines
+15
to
+21
| const result = await wrapErrorHandler( | ||
| signedContract.addSubVault(subVaultAddress), | ||
| 'transaction' | ||
| ) | ||
|
|
||
| return result?.hash | ||
| }) |
* [create-metavault] add metavault creation support * [create-metavault] fixes * [create-metavault] improve types
* add ejectingSubVault and pendingMetaSubVault fields to vault * add new methods to sub-vaults (add, eject, reject)
* Remove sync docs git action (#354) * [remove-sync-docs] remove git action and change script * change git action * add link checker script and update documentation references * fix broken links * add md files to check * small improve * update GitHub Actions to use latest checkout and cache actions (#356) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * add script to check documentation links and update pre-commit hook * update * update * update * add warning for slug changes in staged md files * rename markdown files and update slug checking logic in link checker * Add script to check documentation links (#357) * add script to check documentation links * fix after review * rename documentation files and improve slug validation in checkDocLinks * add category configuration for Quick Start * improve slug processing and include _category_.json in documentation file search * 4.2.3 (#358) * merge --------- Co-authored-by: CAst <programmer.andrey@gmail.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 106 out of 118 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (9)
src/services/vault/transactions/util/check/roles/index.ts:1
- These imports resolve to files inside the new
roles/directory, but the access-check implementations shown in this PR are located insrc/services/vault/transactions/util/check/(one level up). This will break module resolution at build time. Update the import paths (e.g.,../checkAdminAccess, etc.) or move the access-check files into theroles/folder to match the new structure.
src/services/vault/transactions/util/check/checkAccess.ts:1 - The refactor changes other role-check files to import
CheckInputfrom../types, butcheckAccess.tsstill imports from./types, which is inconsistent and likely incorrect after the folder re-org. Also,Actionis hard-coded toPromise<string>, but several wrapped actions in this PR returnstring | undefined(e.g.,result?.hash) or other shapes (encode/estimateGas helpers). MakecheckAccessgeneric over the action output (e.g.,<Output, Input extends CheckInput>(action: (values: Input) => Promise<Output>, ...)) and alignCheckInputimport path with the new types location.
src/services/vault/transactions/util/check/checkAccess.ts:1 - The refactor changes other role-check files to import
CheckInputfrom../types, butcheckAccess.tsstill imports from./types, which is inconsistent and likely incorrect after the folder re-org. Also,Actionis hard-coded toPromise<string>, but several wrapped actions in this PR returnstring | undefined(e.g.,result?.hash) or other shapes (encode/estimateGas helpers). MakecheckAccessgeneric over the action output (e.g.,<Output, Input extends CheckInput>(action: (values: Input) => Promise<Output>, ...)) and alignCheckInputimport path with the new types location.
src/services/vault/transactions/createVault/helpers/index.ts:1 - This barrel export points to
./validateCreateVaultArgsinside thehelpers/directory, but the diff showsvalidateCreateVaultArgs.tsliving insrc/services/vault/transactions/createVault/validateCreateVaultArgs.ts(parent directory), not inhelpers/. As written, this will fail to compile. Either movevalidateCreateVaultArgs.tsintohelpers/, or change the export path to reference the parent file.
src/services/vault/transactions/createVault/common.ts:1 - The conditional type assertion
typeof isMetaVault extends true ? ... : ...won’t narrow based on the runtime booleanisMetaVault(its type isboolean, so this conditional typically collapses to thefalsebranch). Prefer typingvaultFactoryas a union (StakeWise.ABI.VaultFactory | StakeWise.ABI.MetaVaultFactory) and narrowing at runtime withif (isMetaVault) { ... }, or use overloads/generics tied totyperather than a runtime boolean.
src/services/vault/requests/getSubVaults/index.ts:1 getVault()commonly returns nullable metadata fields (e.g.,imageUrlanddisplayNamecan benull). HereOutputSubVaulttypes them as non-nullstring, but the implementation forwards the possibly-null values directly. Update theOutputSubVaulttype to allownullfor these fields or normalize them in the return object to guarantee strings.
src/services/vault/requests/getSubVaults/index.ts:1getVault()commonly returns nullable metadata fields (e.g.,imageUrlanddisplayNamecan benull). HereOutputSubVaulttypes them as non-nullstring, but the implementation forwards the possibly-null values directly. Update theOutputSubVaulttype to allownullfor these fields or normalize them in the return object to guarantee strings.
src/services/vault/requests/getVault/getVault.md:1- The
Outputtype listswhitelistManagertwice, and the file ends with a stray# testline. Remove the duplicate field and the extraneous line to keep the API docs accurate and clean.
src/services/vault/requests/getVault/getVault.md:1 - The
Outputtype listswhitelistManagertwice, and the file ends with a stray# testline. Remove the duplicate field and the extraneous line to keep the API docs accurate and clean.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5
to
+6
| // TODO change | ||
| let network: Network = Network.Hoodi |
There was a problem hiding this comment.
Changing the default codegen network to Hoodi (with a TODO) makes builds/codegen sensitive to local defaults and can accidentally generate types against a staging endpoint. Prefer keeping the default as Mainnet (or requiring an explicit NETWORK env var) so generated outputs are deterministic across environments.
Suggested change
| // TODO change | |
| let network: Network = Network.Hoodi | |
| let network: Network = Network.Mainnet |
| #### Add output field: | ||
| ```ts | ||
| type Output = { | ||
| canHarvest: boolen |
There was a problem hiding this comment.
Corrected spelling of 'boolen' to 'boolean'.
Suggested change
| canHarvest: boolen | |
| canHarvest: boolean |
| sh ./scripts/pre-push.sh | ||
|
|
||
| npm run lint | ||
| pnpm run check:docLinks |
There was a problem hiding this comment.
check:docLinks fetches the live docs sitemap over the network; running this in a pre-commit hook can be slow and may fail for contributors who are offline or behind restrictive networks. Consider moving it to CI (pre-push or workflow) or adding an opt-out (e.g., env flag) so local commits don’t become flaky.
Suggested change
| pnpm run check:docLinks | |
| if [ "${SKIP_DOC_LINKS:-0}" != "1" ]; then | |
| pnpm run check:docLinks | |
| fi |
* [subgraph updates] update sub vaults request Signed-off-by: MikeDiam <info@mdiamond.ru> * [subgraph updates] update cancellable type Signed-off-by: MikeDiam <info@mdiamond.ru> * [subgraph updates] refactor sub vaults query and mapping logic Signed-off-by: MikeDiam <info@mdiamond.ru> --------- Signed-off-by: MikeDiam <info@mdiamond.ru>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 120 out of 132 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (10)
src/services/vault/transactions/util/check/checkAccess.ts:1
CheckInputimports in the other checkers were updated to../types, butcheckAccess.tsstill imports from./types. This will likely break module resolution/compilation iftypeswas moved up one level. Update this import to match the new location (e.g.,../types).
src/services/vault/transactions/updateState/common.ts:1UpdateStateInputis imported as a runtime value import, but it appears to be declared in a.d.tstypes file (types.d.ts). This can cause runtime/bundling errors because there may be no emitted JS module at./types. Change this to a type-only import (import type { UpdateStateInput } from './types'). The same issue appears in other newcommon.tsfiles for sub-vault transactions.
src/services/vault/transactions/addSubVault/common.ts:1AddSubVaultInputis used as a type, but it's imported as a runtime value import. If./typesis a.d.ts-only module, this can break at runtime. Use a type-only import (import type { AddSubVaultInput } from './types'). Apply the same fix inejectSubVault/common.tsandrejectSubVault/common.ts.
src/services/vault/transactions/createVault/common.ts:1- The conditional type cast
typeof isMetaVault extends true ? ...does not work as intended becauseisMetaVaultis a runtime boolean, so the cast will not correctly narrow toMetaVaultFactory. This risks incorrect typing forcreateVaultarguments and can hide real call-site issues. Prefer an explicit runtime branch that selects/casts the factory based onisMetaVault, or use overloads/generics driven by thetypeparameter to ensure the correct ABI type is returned.
src/services/vault/transactions/operate/getRolesList.ts:1 - Role detection uses truthiness checks, which can incorrectly skip required role checks when setting values to falsy-but-valid inputs (e.g.,
feePercent: 0to set fee to 0). Use explicit presence checks (e.g.,feePercent !== undefined) for fields where0, empty string, or empty arrays are meaningful updates.
src/services/vault/requests/getVault/getVault.md:1 - This doc now contains a duplicated
whitelistManagerfield in the output type and an extraneous# testline at the end. Remove the duplicate field and the# testline to avoid confusing SDK consumers and automated doc tooling.
src/services/vault/requests/getVault/getVault.md:1 - This doc now contains a duplicated
whitelistManagerfield in the output type and an extraneous# testline at the end. Remove the duplicate field and the# testline to avoid confusing SDK consumers and automated doc tooling.
src/services/assertCancellable.ts:1 - This file is intended as a compile-time assertion, but it imports many runtime modules and is executed via
src/services/index.ts, which can inflate bundle size and increase risk of circular dependencies. Preferimport typefor the service/transactions classes and structure the assertion so it has zero runtime footprint (e.g., purely type-level aliases or adeclare constpattern) while still failing compilation when methods are not cancellable.
src/services/vault/requests/getSubVaults/modifySubVaults.ts:1 - New request shaping logic is introduced here (BigInt parsing, allocator selection, exitRequests aggregation), but there are no accompanying unit tests in this PR. Add tests covering: (1) empty
allocators, (2) emptyexitRequests, and (3) aggregation across multiple exit requests to ensure stable output formatting.
src/services/vault/transactions/createVault/helpers/getEncodeBytes.ts:1 feePercentis computed via floating point multiplication. Values like15.35 * 100can produce1534.999999...due to IEEE-754 rounding, which can result in an invalid/non-integer ABI value foruint16. Prefer an explicit integer conversion (e.g., rounding to nearest int after validation) to ensure the encoded tuple always contains an integer.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5
to
7
| // TODO change | ||
| let network: Network = Network.Hoodi | ||
|
|
There was a problem hiding this comment.
Changing the default codegen network from Mainnet to Hoodi (and leaving a TODO) can silently generate the wrong GraphQL types/operations when NETWORK is not set (e.g., local dev, CI scripts). Consider restoring Mainnet as the default and making Hoodi opt-in via env/config, or fail fast when NETWORK is missing to avoid accidental wrong builds.
Suggested change
| // TODO change | |
| let network: Network = Network.Hoodi | |
| let network: Network = Network.Mainnet | |
| if (process.env.NETWORK === 'hoodi') { | |
| network = Network.Hoodi | |
| } |
Comment on lines
+42
to
+45
| const output = execSync( | ||
| `grep -r "^slug:" "${srcDir}" --include="*.md"`, | ||
| { encoding: 'utf-8' } | ||
| ) |
There was a problem hiding this comment.
The doc link checker relies on grep, which is not available by default on Windows environments. Since this script is now enforced via Husky on pre-commit, it can block contributors on non-Unix platforms. Consider rewriting the scan using Node filesystem/glob APIs (cross-platform), or run this check only in CI where the environment is controlled.
Comment on lines
+71
to
+76
| const output = execSync( | ||
| `grep -roh "https://docs\\.stakewise\\.io/[^\\"' )\\\`>]*" "${rootDir}" ` | ||
| + '--include="*.ts" --include="*.tsx" --include="*.md" --include="*.mdx" ' | ||
| + '--exclude-dir=node_modules', | ||
| { encoding: 'utf-8' } | ||
| ) |
There was a problem hiding this comment.
The doc link checker relies on grep, which is not available by default on Windows environments. Since this script is now enforced via Husky on pre-commit, it can block contributors on non-Unix platforms. Consider rewriting the scan using Node filesystem/glob APIs (cross-platform), or run this check only in CI where the environment is controlled.
* Remove sync docs git action (#354) * [remove-sync-docs] remove git action and change script * change git action * update GitHub Actions to use latest checkout and cache actions (#356) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Add script to check documentation links (#357) * add script to check documentation links * fix after review * rename documentation files and improve slug validation in checkDocLinks * add category configuration for Quick Start * improve slug processing and include _category_.json in documentation file search * 4.2.3 (#358) * [fix-encode] fix transactionWrapper (#360) * up-version (#361) * [handle-429] add retry-after header support (#364) * fix --------- Co-authored-by: CAst <programmer.andrey@gmail.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 123 out of 135 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (3)
src/services/vault/transactions/createVault/helpers/validateCreateVaultArgs.ts:88
metaVaultvalidation logic is inverted: it returns early whentypeis a MetaVault, so it doesn't enforce the MetaVault restrictions (e.g. disallowingisOwnMevEscrow). Conversely, for non-MetaVault types it can incorrectly throw "MetaVault does not support..." whenisOwnMevEscrowis set. Flip the early-return condition so restrictions are applied only whentypeis a MetaVault/PrivateMetaVault (and do nothing otherwise).
src/services/vault/requests/getVault/getVault.md:115- The return type docs contain a duplicated
whitelistManagerfield and there is a stray# testline at the end of the file. Please remove the duplicate field entry and delete the leftover test text to avoid confusing generated/linked docs.
codegen.ts:13 codegen.tsnow defaults toNetwork.Hoodi(marked TODO). This changes the default behavior ofpnpm graphql/codegen whenNETWORKisn’t set and can generate types against the wrong subgraph/backend in CI or local dev. Consider reverting the default to Mainnet (or requireprocess.env.NETWORKexplicitly and fail fast if unset).
// TODO change
let network: Network = Network.Hoodi
if (process.env.NETWORK === 'mainnet') {
network = Network.Mainnet
}
if (process.env.NETWORK === 'gnosis') {
network = Network.Gnosis
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+13
to
+32
| const modifySubVaults = (data: SubVaultsQueryPayload): ModifiedSubVault[] => { | ||
| return data.subVaults.map(({ subVault }) => { | ||
| const { id, imageUrl, displayName, allocators, exitRequests } = subVault | ||
|
|
||
| const [ allocator ] = allocators | ||
| const exitingAssets = exitRequests.reduce((acc, { totalAssets }) => ( | ||
| acc + BigInt(totalAssets) | ||
| ), 0n) | ||
|
|
||
| const apy = Number(allocator?.apy) || 0 | ||
|
|
||
| return { | ||
| id, | ||
| apy: apy.toFixed(2), | ||
| imageUrl: imageUrl || '', | ||
| displayName: displayName || '', | ||
| stakingAssets: BigInt(allocator?.assets || 0), | ||
| exitingAssets, | ||
| } | ||
| }) |
There was a problem hiding this comment.
modifySubVaults introduces non-trivial parsing logic (BigInt aggregation of exitRequests, allocator selection, APY formatting), but there is no accompanying unit test. The vault request modifiers in this area typically have *.spec.ts coverage; please add a modifySubVaults.spec.ts to lock down edge cases (no allocators, empty exitRequests, large totals).
Comment on lines
19
to
+61
| @@ -28,42 +49,17 @@ export const commonLogic = async (values: CreateVaultTransactionInput) => { | |||
| validateCreateVaultArgs.keysManagerFee(keysManagerFee) | |||
| } | |||
|
|
|||
| const vaultFactories = vaultToken ? { | |||
| [VaultType.Default]: contracts.factories.erc20Vault, | |||
| [VaultType.Private]: contracts.factories.erc20PrivateVault, | |||
| [VaultType.Blocklist]: contracts.factories.erc20BlocklistVault, | |||
| } : { | |||
| [VaultType.Default]: contracts.factories.vault, | |||
| [VaultType.Private]: contracts.factories.privateVault, | |||
| [VaultType.Blocklist]: contracts.factories.blocklistVault, | |||
| } | |||
|
|
|||
| const vaultFactory = vaultFactories[type] | |||
| const defaultAbiCoder = AbiCoder.defaultAbiCoder() | |||
| const encodedParams = getEncodeBytes({ ...values, isMetaVault }) | |||
|
|
|||
| const formattedParams = { | |||
| feePercent: (keysManagerFee || 0) * 100, | |||
| capacity: capacity || MaxUint256, | |||
| symbol: vaultToken?.symbol, | |||
| name: vaultToken?.name, | |||
| } | |||
|
|
|||
| const encodedParams = vaultToken | |||
| ? defaultAbiCoder.encode( | |||
| [ 'tuple(uint256 capacity, uint16 feePercent, string name, string symbol, string metadataIpfsHash)' ], | |||
| [ [ formattedParams.capacity, formattedParams.feePercent, formattedParams.name, formattedParams.symbol, metadataIpfsHash ] ] | |||
| ) | |||
| : defaultAbiCoder.encode( | |||
| [ 'tuple(uint256 capacity, uint16 feePercent, string metadataIpfsHash)' ], | |||
| [ [ formattedParams.capacity, formattedParams.feePercent, metadataIpfsHash ] ] | |||
| ) | |||
|
|
|||
| const isStakeNativeToken = [ Network.Mainnet, Network.Hoodi ].includes(options.network) | |||
|
|
|||
| const params: [ string, boolean, PayableOverrides ] = isStakeNativeToken | |||
| const params: [ string, boolean, PayableOverrides ] = isMainnet | |||
| ? [ encodedParams, isOwnMevEscrow, { value: constants.blockchain.gwei } ] | |||
| : [ encodedParams, isOwnMevEscrow, {} ] | |||
|
|
|||
| if (isMetaVault) { | |||
| // MetaVault does not support isOwnMevEscrow | |||
| params.splice(1, 1) | |||
| } | |||
There was a problem hiding this comment.
The vaultFactory cast uses as typeof isMetaVault extends true ? ... but typeof isMetaVault is just boolean, so the conditional type won’t narrow and effectively hides the real factory type differences. Consider using a proper union/overload keyed off type (VaultType) and make params a discriminated union as well (MetaVault vs non-MetaVault), instead of mutating a fixed tuple via splice.
Comment on lines
+9
to
+14
| validateArgs.address({ vaultAddress, userAddress, subVaultAddress }) | ||
|
|
||
| const { subVaultsRegistry } = await getVault(values) | ||
|
|
||
| return values.contracts.helpers.createSubVaultsRegistry(subVaultsRegistry) | ||
| } |
There was a problem hiding this comment.
getVault() can return subVaultsRegistry as an empty string when the vault isn’t a MetaVault. Since createSubVaultsRegistry('') doesn’t validate the address, this will later fail with a low-level ethers error. Add an explicit check/validation for subVaultsRegistry (and throw a clear error like “subVaultsRegistry is not set; this action is only supported for MetaVaults”).
Comment on lines
+9
to
+14
| validateArgs.address({ vaultAddress, userAddress, subVaultAddress }) | ||
|
|
||
| const { subVaultsRegistry } = await getVault(values) | ||
|
|
||
| return values.contracts.helpers.createSubVaultsRegistry(subVaultsRegistry) | ||
| } |
There was a problem hiding this comment.
getVault() can return subVaultsRegistry as an empty string when the vault isn’t a MetaVault. Since createSubVaultsRegistry('') doesn’t validate the address, this will later fail with a low-level ethers error. Add an explicit check/validation for subVaultsRegistry (and throw a clear error like “subVaultsRegistry is not set; this action is only supported for MetaVaults”).
Comment on lines
+9
to
+14
| validateArgs.address({ vaultAddress, userAddress, subVaultAddress }) | ||
|
|
||
| const { subVaultsRegistry } = await getVault(values) | ||
|
|
||
| return values.contracts.helpers.createSubVaultsRegistry(subVaultsRegistry) | ||
| } |
There was a problem hiding this comment.
getVault() can return subVaultsRegistry as an empty string when the vault isn’t a MetaVault. Since createSubVaultsRegistry('') doesn’t validate the address, this will later fail with a low-level ethers error. Add an explicit check/validation for subVaultsRegistry (and throw a clear error like “subVaultsRegistry is not set; this action is only supported for MetaVaults”).
* add endpoints file * update position * update endpoints docs
* Remove sync docs git action (#354) * [remove-sync-docs] remove git action and change script * change git action * update GitHub Actions to use latest checkout and cache actions (#356) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Add script to check documentation links (#357) * add script to check documentation links * fix after review * rename documentation files and improve slug validation in checkDocLinks * add category configuration for Quick Start * improve slug processing and include _category_.json in documentation file search * 4.2.3 (#358) * [fix-encode] fix transactionWrapper (#360) * up-version (#361) * [handle-429] add retry-after header support (#364) * add context7 public key (#370) * remove old files --------- Co-authored-by: Kadyr Dzhemaledinov <dfkadyr@gmail.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Remove sync docs git action (#354) * [remove-sync-docs] remove git action and change script * change git action * Add script to check documentation links (#357) * add script to check documentation links * fix after review * rename documentation files and improve slug validation in checkDocLinks * add category configuration for Quick Start * improve slug processing and include _category_.json in documentation file search * [context7] test Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] test Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context7.json Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update context Signed-off-by: MikeDiam <info@mdiamond.ru> * [context7] update structure Signed-off-by: MikeDiam <info@mdiamond.ru> --------- Signed-off-by: MikeDiam <info@mdiamond.ru> Co-authored-by: CAst <programmer.andrey@gmail.com> Co-authored-by: Kadyr Dzhemaledinov <dfkadyr@gmail.com>
Signed-off-by: MikeDiam <info@mdiamond.ru>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 163 out of 168 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
src/services/vault/transactions/createVault/helpers/validateCreateVaultArgs.ts:88
metaVault()has inverted logic: it returns early whenisMetaVaultis true, so the meta-vault-specific validations (disallowisOwnMevEscrow, Gnosis ERC20vaultToken, andPrivateMetaVaulton Gnosis) are currently applied to non-meta vaults instead of meta vaults. This will incorrectly block regular vaults withisOwnMevEscrow: trueand fail to block invalid meta vault combinations.
src/services/vault/requests/getVault/getVault.md:115- There is a stray
# testline at the end of the document, which looks accidental and will render into the published docs. Please remove it.
Comment on lines
+19
to
+29
| const isMainnet = [ Network.Mainnet, Network.Hoodi ].includes(options.network) | ||
| const isMetaVault = [ VaultType.MetaVault, VaultType.PrivateMetaVault ].includes(type) | ||
|
|
||
| const vaultFactory = getVaultFactory({ | ||
| ...values, | ||
| isErc20: Boolean(vaultToken), | ||
| vaultType: type, | ||
| }) as typeof isMetaVault extends true | ||
| ? StakeWise.ABI.MetaVaultFactory | ||
| : StakeWise.ABI.VaultFactory | ||
|
|
| @@ -0,0 +1,110 @@ | |||
| import { AbiCoder, MaxUint256, getAddress } from 'ethers' | |||
Comment on lines
45
to
+50
| isSmoothingPool: boolean | ||
| tokenName: string | null | ||
| whitelistManager: string | ||
| whitelistManager: string | ||
| blocklistManager: string | ||
| ejectingSubVault: string |
Comment on lines
+14
to
+21
| const block = () => { | ||
| throw new Error('To send this transaction, please provide BrowserProvider to the StakeWiseSDK') | ||
| }) as T | ||
| } | ||
|
|
||
| const newMethod = block as unknown as T | ||
|
|
||
| newMethod.estimateGas = block as T['estimateGas'] | ||
| newMethod.encode = method.encode |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.