-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
1.0.0 bnc3 backport #1813
1.0.0 bnc3 backport #1813
Conversation
* fix: add coment and delegateUrl params * fix: fix failing test * fix: increase testing time --------- Co-authored-by: Fabio Rigamonti <73019897+fabiorigam@users.noreply.github.com> Co-authored-by: Miguel Angel Rojo <freemanz1486@gmail.com>
* 1595 refactor the transaction class using only the concept of sender and gas payer (#1596) * refactor: 1595 Transaction.ts refactoring... * refactor: 1595 Transaction.ts refactoring... * refactor: 1595 Transaction.ts refactoring... * refactor: 1595 Transaction.ts refactoring... * refactor: 1596 examples fixed --------- Co-authored-by: Fabio Rigamonti <73019897+fabiorigam@users.noreply.github.com> * chore: fix test --------- Co-authored-by: Luca Nicola Debiasi <63785793+lucanicoladebiasi@users.noreply.github.com> Co-authored-by: Fabio Rigamonti <73019897+fabiorigam@users.noreply.github.com>
Bumps [@aws-sdk/client-kms](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-kms) from 3.696.0 to 3.723.0. - [Release notes](https://github.com/aws/aws-sdk-js-v3/releases) - [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-kms/CHANGELOG.md) - [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.723.0/clients/client-kms) --- updated-dependencies: - dependency-name: "@aws-sdk/client-kms" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* chore: update dependencies * chore: minor html fix
Bumps [vitest-browser-react](https://github.com/vitest-dev/vitest-browser-react) from 0.0.3 to 0.0.4. - [Release notes](https://github.com/vitest-dev/vitest-browser-react/releases) - [Commits](vitest-dev/vitest-browser-react@v0.0.3...v0.0.4) --- updated-dependencies: - dependency-name: vitest-browser-react dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [eslint-plugin-promise](https://github.com/eslint-community/eslint-plugin-promise) from 7.1.0 to 7.2.1. - [Release notes](https://github.com/eslint-community/eslint-plugin-promise/releases) - [Changelog](https://github.com/eslint-community/eslint-plugin-promise/blob/main/CHANGELOG.md) - [Commits](eslint-community/eslint-plugin-promise@v7.1.0...v7.2.1) --- updated-dependencies: - dependency-name: eslint-plugin-promise dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Fabio Rigamonti <73019897+fabiorigam@users.noreply.github.com>
Bumps [eslint-plugin-n](https://github.com/eslint-community/eslint-plugin-n) from 17.13.1 to 17.15.1. - [Release notes](https://github.com/eslint-community/eslint-plugin-n/releases) - [Changelog](https://github.com/eslint-community/eslint-plugin-n/blob/master/CHANGELOG.md) - [Commits](eslint-community/eslint-plugin-n@v17.13.1...v17.15.1) --- updated-dependencies: - dependency-name: eslint-plugin-n dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Fabio Rigamonti <73019897+fabiorigam@users.noreply.github.com>
Bumps [@cloudflare/workers-types](https://github.com/cloudflare/workerd) from 4.20241112.0 to 4.20250109.0. - [Release notes](https://github.com/cloudflare/workerd/releases) - [Changelog](https://github.com/cloudflare/workerd/blob/main/RELEASE.md) - [Commits](https://github.com/cloudflare/workerd/commits) --- updated-dependencies: - dependency-name: "@cloudflare/workers-types" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Fabio Rigamonti <73019897+fabiorigam@users.noreply.github.com>
Bumps [@vitest/browser](https://github.com/vitest-dev/vitest/tree/HEAD/packages/browser) from 2.1.5 to 2.1.8. - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Commits](https://github.com/vitest-dev/vitest/commits/v2.1.8/packages/browser) --- updated-dependencies: - dependency-name: "@vitest/browser" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Fabio Rigamonti <73019897+fabiorigam@users.noreply.github.com>
* chore: rpc docs * chore: rpc dockerfile * chore: node env * chore: ignore cve * chore: production packages * chore: update comment * chore: code review * chore: code review * chore: fix * chore: fix * Revert "chore: fix" This reverts commit b20a93c. * chore: fix * chore: add back trivy ignore
* build(deps-dev): bump @eslint/js from 9.13.0 to 9.18.0 Bumps [@eslint/js](https://github.com/eslint/eslint/tree/HEAD/packages/js) from 9.13.0 to 9.18.0. - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/main/CHANGELOG.md) - [Commits](https://github.com/eslint/eslint/commits/v9.18.0/packages/js) --- updated-dependencies: - dependency-name: "@eslint/js" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * fix: fix undici version --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Fabio Rigamonti <73019897+fabiorigam@users.noreply.github.com>
Bumps [@scure/bip39](https://github.com/paulmillr/scure-bip39) from 1.4.0 to 1.5.1. - [Release notes](https://github.com/paulmillr/scure-bip39/releases) - [Commits](paulmillr/scure-bip39@1.4.0...1.5.1) --- updated-dependencies: - dependency-name: "@scure/bip39" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Fabio Rigamonti <73019897+fabiorigam@users.noreply.github.com>
Bumps [prettier](https://github.com/prettier/prettier) from 3.4.1 to 3.4.2. - [Release notes](https://github.com/prettier/prettier/releases) - [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md) - [Commits](prettier/prettier@3.4.1...3.4.2) --- updated-dependencies: - dependency-name: prettier dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Fabio Rigamonti <73019897+fabiorigam@users.noreply.github.com>
) Bumps [eslint-config-prettier](https://github.com/prettier/eslint-config-prettier) from 9.1.0 to 10.0.1. - [Release notes](https://github.com/prettier/eslint-config-prettier/releases) - [Changelog](https://github.com/prettier/eslint-config-prettier/blob/main/CHANGELOG.md) - [Commits](prettier/eslint-config-prettier@v9.1.0...v10.0.1) --- updated-dependencies: - dependency-name: eslint-config-prettier dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Fabio Rigamonti <73019897+fabiorigam@users.noreply.github.com>
(cherry picked from commit c4504bc)
(cherry picked from commit 98ca258)
* fix: fix BNC... * fix: fix BNC... * fix: fix BNC... * fix: fix BNC... * fix: fix BNC... * fix: fix BNC... (cherry picked from commit 3eecea9)
* fix: fr9 recommendations * fix: en_1 recommendations * fix: en_1 recommendations * fix: en_2 recommendations... * fix: en_2 recommendations... * fix: en_7 recommendations... * fix: en_7 recommendations... * fix: n_7 recommandations * fix: n_7 recommandations * Merge branch 'main' into 1656-fr9 * Merge branch '1661-en_2' into 1662-en_7 * fix: EN_7 * fix: EN_7 * fix: EN_7 --------- Co-authored-by: Fabio Rigamonti <73019897+fabiorigam@users.noreply.github.com> (cherry picked from commit f52a556)
WalkthroughThe changes update GitHub Actions workflows to trigger on an expanded set of branch patterns, update package versions and dependency versions from 1.0.0-rc.3 to 1.0.0-rc.6 across multiple apps and packages, and adjust TypeScript configuration settings to target Node16 and ES2023. Import paths and file extensions have been revised, and the naming convention is changed from “delegator” to “gasPayer” in configurations, documentation, tests, and signing modules. Additionally, new functions and scripts for handling block revisions and package adjustments are introduced, and test suites are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as VeChainPrivateKeySigner
participant F as _signFlow (Gas Payer Flow)
participant N as Network (thorClient)
U->>S: Submit transaction request
S->>F: Process signing with gasPayer info
F->>S: Return signed transaction
S->>N: Send signed transaction
N-->>S: Transaction confirmed
S-->>U: Return transaction receipt
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🔭 Outside diff range comments (3)
packages/network/src/http/SimpleHttpClient.ts (1)
108-108
:⚠️ Potential issueFix incorrect headers usage in fetch call.
The fetch call is using
params?.headers
directly instead of the merged headers fromheaders
variable.Apply this fix:
- headers: params?.headers as HeadersInit, + headers,packages/network/src/provider/utils/rpc-mapper/methods/eth_getBlockByNumber/eth_getBlockByNumber.ts (1)
34-36
:⚠️ Potential issueUpdate type check to match new parameter type.
The type check still validates
params[0]
asstring
but the parameter type has been updated toDefaultBlock
.Apply this diff to fix the type validation:
- typeof params[0] !== 'string' || + !params[0] || // Check for null/undefineddocs/diagrams/architecture/transaction.md (1)
9-9
:⚠️ Potential issueFix typo in method name.
The method name has a typo:
Cluase
should beClause
.- +Cluase transferVET(Address recipientAddress, VET amount, ClauseOptions clauseOptions?) + +Clause transferVET(Address recipientAddress, VET amount, ClauseOptions clauseOptions?)
🧹 Nitpick comments (24)
packages/network/tests/http/SimpleHttpClient.testnet.test.ts (1)
55-59
: Enhance test coverage for URL path handling.The current tests only verify that responses are defined. Consider enhancing the test coverage by:
- Validating the actual response data structure and content
- Adding assertions to verify the URL construction
- Including error cases (e.g., malformed paths)
Here's a suggested improvement:
test('Test http without leading slash', async () => { const httpClient = new SimpleHttpClient(TESTNET_URL); const resp = await httpClient.http(HttpMethod.GET, 'blocks/best'); - expect(resp).toBeDefined(); + expect(resp).toMatchObject({ + number: expect.any(Number), + id: expect.stringMatching(/^0x[a-f0-9]+$/), + parentID: expect.stringMatching(/^0x[a-f0-9]+$/), + timestamp: expect.any(Number), + }); }); test('Test http with leading slash', async () => { const httpClient = new SimpleHttpClient(TESTNET_URL); const resp = await httpClient.http(HttpMethod.GET, '/blocks/best'); - expect(resp).toBeDefined(); + expect(resp).toMatchObject({ + number: expect.any(Number), + id: expect.stringMatching(/^0x[a-f0-9]+$/), + parentID: expect.stringMatching(/^0x[a-f0-9]+$/), + timestamp: expect.any(Number), + }); }); +test('Test http with malformed path', async () => { + const httpClient = new SimpleHttpClient(TESTNET_URL); + await expect( + httpClient.http(HttpMethod.GET, '//blocks//best') + ).rejects.toThrow(InvalidHTTPRequest); +});Also applies to: 61-65
packages/network/src/http/SimpleHttpClient.ts (2)
85-94
: Improve URL construction robustness.The current URL construction logic could be more robust by:
- Handling multiple consecutive slashes
- Handling empty paths
- Using URL manipulation utilities
Consider this improvement:
- // Remove leading slash from path - if (path.startsWith('/')) { - path = path.slice(1); - } - // Add trailing slash from baseURL if not present - let baseURL = this.baseURL; - if (!this.baseURL.endsWith('/')) { - baseURL += '/'; - } - const url = new URL(path, baseURL); + // Normalize path by removing multiple consecutive slashes and ensuring no leading slash + const normalizedPath = path.replace(/\/+/g, '/').replace(/^\//, ''); + if (!normalizedPath) { + throw new InvalidHTTPRequest( + 'HttpClient.http()', + 'Path cannot be empty', + { method, url: this.baseURL } + ); + } + // Ensure baseURL ends with exactly one slash + const normalizedBaseURL = this.baseURL.replace(/\/+$/, '') + '/'; + const url = new URL(normalizedPath, normalizedBaseURL);
137-137
: Update error URL construction.The error message URL construction should use the normalized baseURL and path.
Apply this fix:
- url: `${this.baseURL}${path}` + url: url.toString()packages/network/src/provider/utils/rpc-mapper/methods/eth_getBlockByHash/eth_getBlockByHash.ts (1)
43-50
: Parameter naming could be more descriptive.Using
const [blockHash, isTxDetail]
is fine but consider a clearer variable name (e.g.,includeTxDetails
) to better reflect its boolean usage. This minor change can improve readability in the long run.- const [blockHash, isTxDetail] = params as [string, boolean]; + const [blockHash, includeTxDetails] = params as [string, boolean];apps/sdk-vite-integration/tsconfig.app.json (1)
9-9
: Remove comments from JSON file.JSON standard does not allow comments. Consider moving the comments to a separate documentation file or using JSONC/JSON5 if comments are necessary.
Apply this diff to remove the comments:
- /* Bundler mode */ - /* Linting */Also applies to: 17-17
🧰 Tools
🪛 Biome (1.9.4)
[error] 9-9: JSON standard does not allow comments.
(parse)
apps/sdk-vite-integration/src/components/Hash.tsx (1)
3-3
: Useimport type
for type-only imports.Since
HashedContent
is only used as a type, it's recommended to useimport type
for better code organization and potential performance benefits.Apply this diff:
-import { HashedContent } from '../types/index.tsx'; +import type { HashedContent } from '../types/index.tsx';🧰 Tools
🪛 ESLint
[error] 3-3: All imports in the declaration are only used as types. Use
import type
.(@typescript-eslint/consistent-type-imports)
apps/sdk-vite-integration/src/components/TransferLogs.tsx (1)
8-9
: Update import statements to follow TypeScript best practices.
- Use
import type
for type-only imports- Use single quotes for consistency with the codebase style
-import { Transfer } from "../types/index.tsx"; -import { explorerUrl, thorClient } from "../const/index.tsx"; +import type { Transfer } from '../types/index.tsx'; +import { explorerUrl, thorClient } from '../const/index.tsx';🧰 Tools
🪛 ESLint
[error] 8-8: All imports in the declaration are only used as types. Use
import type
.(@typescript-eslint/consistent-type-imports)
[error] 8-8: Replace
"../types/index.tsx"
with'../types/index.tsx'
(prettier/prettier)
[error] 9-9: Replace
"../const/index.tsx"
with'../const/index.tsx'
(prettier/prettier)
packages/core/jest.config.js (1)
11-11
: Coverage threshold decreased.
You lowered branch coverage from 95% to 94%. If feasible, consider adding more tests to maintain or exceed 95%.docs/examples/contracts/contract-clauses.ts (1)
21-24
: Variable unused in example code.
transferVTHOClause
is assigned but never referenced. If this snippet is purely illustrative, consider removing or using it within the example for clarity.🧰 Tools
🪛 ESLint
[error] 21-21: 'transferVTHOClause' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
packages/network/tests/provider/rpc-mapper/methods/eth_getBlockTransactionCountByNumber/fixture.ts (1)
3-10
: Consider parameterizing the block number test case.The hardcoded block number
0x13F6730
might be environment-dependent. Consider parameterizing this value or documenting why this specific block number was chosen.packages/network/src/provider/utils/rpc-mapper/methods/eth_getBlockTransactionCountByNumber/eth_getBlockTransactionCountByNumber.ts (1)
29-29
: Consider performance implications of fetching full transaction objects.The change from
false
totrue
inethGetBlockByNumber
means full transaction objects will be fetched instead of just transaction hashes. While this doesn't affect the final result (transaction count), it could impact performance unnecessarily.Consider keeping it as
false
since only the transaction count is needed, not the full transaction details.- const block = await ethGetBlockByNumber(thorClient, [params[0], true]); + const block = await ethGetBlockByNumber(thorClient, [params[0], false]);docs/examples/transactions/fee-delegation.ts (1)
64-64
: LGTM! The terminology changes improve clarity.The renaming from "delegator" to "gasPayer" better reflects the role in the transaction flow. The method name
signAsSenderAndGasPayer
is also more explicit about the signing process.Consider adding a comment explaining that the gas payer is responsible for covering the transaction fees, to help new users understand the role better.
Also applies to: 68-68, 72-74, 88-88
packages/network/src/provider/utils/rpc-mapper/methods/eth_call/eth_call.ts (1)
6-6
: LGTM! The block parameter handling improvements enhance type safety.The changes standardize block parameter handling by:
- Using the
DefaultBlock
type for block parameters- Using
DefaultBlockToRevision
for consistent block revision conversionConsider using a type assertion in the params destructuring to improve type safety:
- const [inputOptions, block] = params as [ - TransactionObjectInput, - DefaultBlock - ]; + const [inputOptions, block] = params as readonly [TransactionObjectInput, DefaultBlock];Also applies to: 43-43, 56-56
packages/network/tests/provider/rpc-mapper/methods/eth_estimateGas/fixture.ts (1)
55-64
: Add more descriptive test documentation.The test case would benefit from more detailed documentation explaining why missing block reference is considered an invalid case.
Add a more detailed description:
- description: 'Missing block reference', + description: 'Missing block reference - should fail as block reference is required for gas estimation',docs/examples/transactions/full-flow-delegator-url.ts (2)
57-60
: Consider updating the configuration object key.Since "delegator" is being deprecated in favor of "gasPayer", consider updating the configuration object key as well.
Apply this diff to align with the new terminology:
- delegator: { - delegatorUrl: gasPayerAccount.URL + gasPayer: { + gasPayerUrl: gasPayerAccount.URL
127-127
: Remove commented-out code.The commented-out expectation check should be either removed or updated to use the new terminology.
Either remove the line or update it to use the new terminology:
-// expect(signedTx.delegator).toEqual(gasPayerAccount.address); --- +expect(signedTx.gasPayer).toEqual(gasPayerAccount.address);docs/examples/transactions/full-flow-delegator-private-key.ts (1)
53-56
: Consider updating the configuration keys to use the new terminology.While the comment indicates that "delegator" will be deprecated, the configuration still uses the old terminology (
delegator
,delegatorPrivateKey
). Consider updating these to match the new gas-payer terminology for consistency.- delegator: { - delegatorPrivateKey: gasPayerAccount.privateKey + gasPayer: { + gasPayerPrivateKey: gasPayerAccount.privateKeypackages/errors/src/available-errors/provider/provider.ts (1)
121-127
: Consider enhancing the error class with additional context.The
JSONRPCInvalidDefaultBlock
error class could benefit from:
- A custom constructor to format error messages consistently
- A more specific data type than just
string
to include block details-class JSONRPCInvalidDefaultBlock extends VechainSDKError<string> {} +interface InvalidDefaultBlockData { + providedValue: string; + allowedValues: string[]; +} + +class JSONRPCInvalidDefaultBlock extends VechainSDKError<InvalidDefaultBlockData> { + constructor( + methodName: string, + providedValue: string, + allowedValues: string[], + innerError?: unknown + ) { + const message = `Invalid default block: ${providedValue}. Allowed values: ${allowedValues.join(', ')}`; + super(methodName, message, { providedValue, allowedValues }, innerError); + } +}packages/network/src/thor-client/transactions/helpers/delegation-handler.ts (1)
49-51
: Update error message to use consistent terminology.While the property has been correctly renamed to
gasPayerUrl
, the error message still uses the old "delegator" terminology.Apply this diff to maintain terminology consistency:
- 'Delegation failed: Cannot get signature from delegator.', + 'Delegation failed: Cannot get signature from gas payer.', { gasPayerUrl: delegatorUrl },packages/core/src/vcdm/Address.ts (1)
94-106
: Security improvements in error handling.The changes enhance security by:
- Always using compressed public keys (hardcoded to
true
).- Obfuscating private key data in error messages.
Consider adding a comment explaining why compression is always enabled, for future maintainers:
public static ofPrivateKey(privateKey: Uint8Array): Address { try { + // Always use compressed public keys for consistency and efficiency return Address.ofPublicKey( Secp256k1.derivePublicKey(privateKey, true) );
packages/core/src/hdkey/HDKey.ts (1)
237-246
: Consider making the BIP32 path validation more flexible.The current regex pattern
^m(\/\d+'?){3}(\/\d+){1,2}$
enforces:
- Exactly 3 hardened derivation levels (
/\d+'
)- 1-2 non-hardened levels (
/\d+
)This might be too restrictive for some valid BIP32 paths.
Consider using a more flexible pattern that allows variable hardened and non-hardened levels:
-const bip32Regex = /^m(\/\d+'?){3}(\/\d+){1,2}$/; +const bip32Regex = /^m(\/\d+'?)*$/;packages/network/src/thor-client/transactions/transactions-module.ts (1)
783-786
: Document the new transaction parameters.The new parameters
delegationUrl
andcomment
have been added but lack documentation explaining their purpose and usage.Add JSDoc comments for the new parameters:
* @param {ContractTransactionOptions} [options] - Optional transaction parameters. + * @param {string} [options.delegationUrl] - URL for transaction delegation service. + * @param {string} [options.comment] - Optional comment to attach to the transaction. * @return {Promise<SendTransactionResult>} - A promise that resolves to the result of the transaction..github/workflows/gitleaks.yml (1)
5-9
: Fix indentation for consistency.The indentation of branch patterns should be 10 spaces to match YAML conventions.
- - 'main' - - 'main-*' - - 'supported-*' - - 'feature-*' - - '1.0.0' + - 'main' + - 'main-*' + - 'supported-*' + - 'feature-*' + - '1.0.0'🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 5-5: wrong indentation: expected 10 but found 8
(indentation)
docker/rpc-proxy/adjust-packages.sh (1)
26-46
: Add backup functionality before modifying files.Consider creating backups before modifying package.json files to allow for recovery if needed.
find "$SEARCH_DIR" -type f -name "package.json" ! -path "*/node_modules/*" | while read PACKAGE_JSON; do echo "Processing: $PACKAGE_JSON" + + # Create backup + cp "$PACKAGE_JSON" "${PACKAGE_JSON}.bak" + echo "Backup created: ${PACKAGE_JSON}.bak" # Remove the "devDependencies" section and overwrite the file if jq -e '.devDependencies' "$PACKAGE_JSON" > /dev/null; then
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (107)
.github/workflows/gitleaks.yml
(1 hunks).github/workflows/on-main.yml
(1 hunks).github/workflows/on-pr.yml
(1 hunks)apps/sdk-cloudflare-integration/package.json
(2 hunks)apps/sdk-cloudflare-integration/tsconfig.json
(2 hunks)apps/sdk-hardhat-integration/hardhat.config.ts
(8 hunks)apps/sdk-hardhat-integration/package.json
(2 hunks)apps/sdk-nextjs-integration/package.json
(3 hunks)apps/sdk-nextjs-integration/src/app/transfer-logs/page.tsx
(0 hunks)apps/sdk-nextjs-integration/tsconfig.json
(2 hunks)apps/sdk-node-integration/package.json
(2 hunks)apps/sdk-vite-integration/package.json
(3 hunks)apps/sdk-vite-integration/src/App.tsx
(1 hunks)apps/sdk-vite-integration/src/components/GetLastBlock.tsx
(2 hunks)apps/sdk-vite-integration/src/components/Hash.tsx
(1 hunks)apps/sdk-vite-integration/src/components/TransferLogs.tsx
(1 hunks)apps/sdk-vite-integration/src/const/index.tsx
(1 hunks)apps/sdk-vite-integration/src/main.tsx
(1 hunks)apps/sdk-vite-integration/src/types/index.tsx
(1 hunks)apps/sdk-vite-integration/tests/Hash.spec.tsx
(1 hunks)apps/sdk-vite-integration/tsconfig.app.json
(1 hunks)apps/sdk-vite-integration/tsconfig.json
(1 hunks)apps/sdk-vite-integration/tsconfig.node.json
(1 hunks)docker/rpc-proxy/Dockerfile
(1 hunks)docker/rpc-proxy/adjust-packages.sh
(1 hunks)docs/contracts.md
(3 hunks)docs/diagrams/architecture/transaction.md
(1 hunks)docs/examples/contracts/contract-clauses.ts
(1 hunks)docs/examples/contracts/contract-delegation-ERC20.ts
(4 hunks)docs/examples/thor-client/delegated-transactions.ts
(1 hunks)docs/examples/transactions/fee-delegation.ts
(2 hunks)docs/examples/transactions/full-flow-delegator-private-key.ts
(3 hunks)docs/examples/transactions/full-flow-delegator-url.ts
(3 hunks)docs/examples/transactions/multiple-clauses.ts
(1 hunks)docs/package.json
(2 hunks)docs/templates/contracts.md
(1 hunks)docs/thor-client.md
(1 hunks)docs/transactions.md
(6 hunks)docs/tsconfig.json
(1 hunks)package.json
(2 hunks)packages/aws-kms-adapter/package.json
(2 hunks)packages/aws-kms-adapter/tests/KMSVeChainSigner.solo.test.ts
(1 hunks)packages/core/jest.config.js
(1 hunks)packages/core/package.json
(2 hunks)packages/core/src/hdkey/HDKey.ts
(1 hunks)packages/core/src/secp256k1/Secp256k1.ts
(3 hunks)packages/core/src/transaction/Clause.ts
(6 hunks)packages/core/src/transaction/Transaction.ts
(10 hunks)packages/core/src/vcdm/Address.ts
(2 hunks)packages/core/src/vcdm/Revision.ts
(3 hunks)packages/core/src/vcdm/hash/Keccak256.ts
(1 hunks)packages/core/tests/hdkey/HDKey.unit.test.ts
(2 hunks)packages/core/tests/keystore/keystore.unit.test.ts
(2 hunks)packages/core/tests/secp256k1/Secp256k1.unit.test.ts
(1 hunks)packages/core/tests/secp256k1/fixture.ts
(0 hunks)packages/core/tests/transaction/Clause.unit.test.ts
(6 hunks)packages/core/tests/transaction/Transaction.unit.test.ts
(18 hunks)packages/core/tests/vcdm/Address.unit.test.ts
(4 hunks)packages/core/tests/vcdm/Revision.unit.test.ts
(2 hunks)packages/errors/package.json
(1 hunks)packages/errors/src/available-errors/provider/provider.ts
(2 hunks)packages/errors/src/available-errors/transaction/transaction.ts
(1 hunks)packages/errors/src/helpers/helpers.ts
(1 hunks)packages/errors/tests/available-errors/transaction.unit.test.ts
(2 hunks)packages/errors/tests/helpers/helpers.unit.test.ts
(1 hunks)packages/ethers-adapter/package.json
(2 hunks)packages/hardhat-plugin/package.json
(2 hunks)packages/hardhat-plugin/tests/helpers/fixture.ts
(3 hunks)packages/logging/package.json
(2 hunks)packages/network/package.json
(2 hunks)packages/network/solo-seeding/const.ts
(0 hunks)packages/network/solo-seeding/index.ts
(0 hunks)packages/network/solo-seeding/thor-solo-seeding.ts
(0 hunks)packages/network/src/http/SimpleHttpClient.ts
(2 hunks)packages/network/src/provider/providers/hardhat-provider/hardhat-provider.ts
(1 hunks)packages/network/src/provider/providers/vechain-provider/vechain-provider.ts
(1 hunks)packages/network/src/provider/utils/const/blocks/blocks.ts
(1 hunks)packages/network/src/provider/utils/rpc-mapper/index.ts
(0 hunks)packages/network/src/provider/utils/rpc-mapper/methods/eth_call/eth_call.ts
(3 hunks)packages/network/src/provider/utils/rpc-mapper/methods/eth_estimateGas/eth_estimateGas.ts
(4 hunks)packages/network/src/provider/utils/rpc-mapper/methods/eth_getBalance/eth_getBalance.ts
(2 hunks)packages/network/src/provider/utils/rpc-mapper/methods/eth_getBlockByHash/eth_getBlockByHash.ts
(3 hunks)packages/network/src/provider/utils/rpc-mapper/methods/eth_getBlockByNumber/eth_getBlockByNumber.ts
(3 hunks)packages/network/src/provider/utils/rpc-mapper/methods/eth_getBlockTransactionCountByNumber/eth_getBlockTransactionCountByNumber.ts
(1 hunks)packages/network/src/provider/utils/rpc-mapper/methods/eth_getCode/eth_getCode.ts
(2 hunks)packages/network/src/provider/utils/rpc-mapper/methods/eth_getStorageAt/eth_getStorageAt.ts
(2 hunks)packages/network/src/provider/utils/rpc-mapper/methods/eth_getTransactionByHash/eth_getTransactionByHash.ts
(2 hunks)packages/network/src/provider/utils/rpc-mapper/rpc-mapper.ts
(1 hunks)packages/network/src/provider/utils/rpc-mapper/types.d.ts
(0 hunks)packages/network/src/signer/signers/types.d.ts
(1 hunks)packages/network/src/signer/signers/vechain-abstract-signer/vechain-abstract-signer.ts
(2 hunks)packages/network/src/signer/signers/vechain-private-key-signer/vechain-private-key-signer.ts
(5 hunks)packages/network/src/thor-client/blocks/blocks-module.ts
(1 hunks)packages/network/src/thor-client/blocks/types.d.ts
(0 hunks)packages/network/src/thor-client/contracts/types.d.ts
(1 hunks)packages/network/src/thor-client/transactions/helpers/delegation-handler.ts
(1 hunks)packages/network/src/thor-client/transactions/transactions-module.ts
(2 hunks)packages/network/tests/http/SimpleHttpClient.testnet.test.ts
(2 hunks)packages/network/tests/provider/formatter/fixture.ts
(0 hunks)packages/network/tests/provider/rpc-mapper/methods/eth_call/eth_call.mock.solo.test.ts
(2 hunks)packages/network/tests/provider/rpc-mapper/methods/eth_call/fixture.ts
(1 hunks)packages/network/tests/provider/rpc-mapper/methods/eth_estimateGas/fixture.ts
(1 hunks)packages/network/tests/provider/rpc-mapper/methods/eth_getBalance/fixture.ts
(0 hunks)packages/network/tests/provider/rpc-mapper/methods/eth_getBlockByHash/fixture.ts
(1 hunks)packages/network/tests/provider/rpc-mapper/methods/eth_getBlockTransactionCountByNumber/eth_getBlockTransactionCountByNumber.testnet.test.ts
(3 hunks)packages/network/tests/provider/rpc-mapper/methods/eth_getBlockTransactionCountByNumber/fixture.ts
(2 hunks)packages/network/tests/seeding/seeding.solo.test.ts
(0 hunks)
⛔ Files not processed due to max files limit (26)
- packages/network/tests/signer/signers/vechain-private-key-signer/vechain-private-key-signer.testnet.test.ts
- packages/network/tests/signer/signers/vechain-private-key-signer/vechain-private-key-signer.unit.test.ts
- packages/network/tests/thor-client/blocks/fixture.ts
- packages/network/tests/thor-client/contracts/contract.erc20.testnet.test.ts
- packages/network/tests/thor-client/contracts/contract.event.solo.test.ts
- packages/network/tests/thor-client/contracts/contract.solo.test.ts
- packages/network/tests/thor-client/transactions/transactions-thorest.solo.test.ts
- packages/rpc-proxy/README.md
- packages/rpc-proxy/package.json
- packages/rpc-proxy/src/utils/args-validator/args-validator-and-getter.ts
- packages/rpc-proxy/src/utils/args-validator/args-validator.ts
- packages/rpc-proxy/src/utils/args/args-options.ts
- packages/rpc-proxy/src/utils/args/args-parser.ts
- packages/rpc-proxy/src/utils/args/env-to-args.ts
- packages/rpc-proxy/src/utils/config-validator/config-validator.ts
- packages/rpc-proxy/src/utils/validators/validators.ts
- packages/rpc-proxy/tests/config-files-fixtures/correct-proxy-config-delegator-private-key.json
- packages/rpc-proxy/tests/config-files-fixtures/correct-proxy-config-delegator-url.json
- packages/rpc-proxy/tests/e2e_rpc_proxy.solo.test.ts
- packages/rpc-proxy/tests/fixture.ts
- packages/rpc-proxy/tests/utils/args/args-options.unit.test.ts
- packages/rpc-proxy/tests/utils/args/args-parser.unit.test.ts
- packages/rpc-proxy/tests/utils/args/env-to-args-positive-cases.unit.test.ts
- packages/rpc-proxy/tests/utils/config-validator/config-validator.unit.test.ts
- packages/rpc-proxy/tsconfig.json
- tsconfig.json
💤 Files with no reviewable changes (11)
- packages/network/src/thor-client/blocks/types.d.ts
- apps/sdk-nextjs-integration/src/app/transfer-logs/page.tsx
- packages/network/tests/seeding/seeding.solo.test.ts
- packages/network/src/provider/utils/rpc-mapper/types.d.ts
- packages/network/tests/provider/rpc-mapper/methods/eth_getBalance/fixture.ts
- packages/network/solo-seeding/index.ts
- packages/network/tests/provider/formatter/fixture.ts
- packages/network/solo-seeding/const.ts
- packages/network/solo-seeding/thor-solo-seeding.ts
- packages/network/src/provider/utils/rpc-mapper/index.ts
- packages/core/tests/secp256k1/fixture.ts
✅ Files skipped from review due to trivial changes (13)
- packages/network/src/provider/providers/hardhat-provider/hardhat-provider.ts
- apps/sdk-vite-integration/src/const/index.tsx
- packages/errors/package.json
- docs/package.json
- packages/logging/package.json
- apps/sdk-vite-integration/src/types/index.tsx
- apps/sdk-vite-integration/src/App.tsx
- apps/sdk-cloudflare-integration/package.json
- apps/sdk-hardhat-integration/package.json
- packages/ethers-adapter/package.json
- docs/templates/contracts.md
- packages/hardhat-plugin/package.json
- apps/sdk-node-integration/package.json
🧰 Additional context used
🧠 Learnings (7)
packages/network/src/thor-client/transactions/helpers/delegation-handler.ts (1)
Learnt from: lucanicoladebiasi
PR: vechain/vechain-sdk-js#1777
File: packages/errors/src/available-errors/transaction/transaction.ts:33-33
Timestamp: 2025-01-30T17:15:49.377Z
Learning: PR #1777 is part of a phased approach to rename "delegator" terminology to "gas-payer" across the VeChain SDK. The changes are intentionally partial, with remaining occurrences to be addressed in subsequent PRs.
packages/hardhat-plugin/tests/helpers/fixture.ts (3)
Learnt from: lucanicoladebiasi
PR: vechain/vechain-sdk-js#1788
File: packages/hardhat-plugin/tests/hardhat-mock-projects/eth_getTransactionCount-no-value-project/hardhat.config.ts:32-35
Timestamp: 2025-02-07T16:04:18.593Z
Learning: The parent object `delegator` in hardhat configuration files is intentionally left unchanged during the current BNC refactoring phase, with only its inner properties being renamed (e.g., `delegatorPrivateKey` to `gasPayerPrivateKey`). The parent object will be renamed to `gasPayer` in a future phase.
Learnt from: lucanicoladebiasi
PR: vechain/vechain-sdk-js#1788
File: packages/hardhat-plugin/tests/hardhat-mock-projects/eth_getTransactionCount-default-value-project/hardhat.config.ts:0-0
Timestamp: 2025-02-07T15:28:09.555Z
Learning: The `delegator` property in hardhat configuration files is planned to be renamed to `gasPayer` in a future BNC refactoring phase. Until then, it contains the `gasPayerPrivateKey` property.
Learnt from: lucanicoladebiasi
PR: vechain/vechain-sdk-js#1788
File: packages/hardhat-plugin/tests/hardhat-mock-projects/eth_getTransactionCount-random-value-project/hardhat.config.ts:0-0
Timestamp: 2025-02-07T15:29:28.678Z
Learning: The renaming of 'delegator' to 'gasPayer' in configuration files is planned for the next refactoring phase of the project.
docs/examples/transactions/full-flow-delegator-url.ts (1)
Learnt from: lucanicoladebiasi
PR: vechain/vechain-sdk-js#1777
File: packages/errors/src/available-errors/transaction/transaction.ts:33-33
Timestamp: 2025-01-30T17:15:49.377Z
Learning: PR #1777 is part of a phased approach to rename "delegator" terminology to "gas-payer" across the VeChain SDK. The changes are intentionally partial, with remaining occurrences to be addressed in subsequent PRs.
packages/errors/src/available-errors/transaction/transaction.ts (1)
Learnt from: lucanicoladebiasi
PR: vechain/vechain-sdk-js#1777
File: packages/errors/src/available-errors/transaction/transaction.ts:33-33
Timestamp: 2025-01-30T17:15:49.377Z
Learning: PR #1777 is part of a phased approach to rename "delegator" terminology to "gas-payer" across the VeChain SDK. The changes are intentionally partial, with remaining occurrences to be addressed in subsequent PRs.
packages/errors/tests/available-errors/transaction.unit.test.ts (1)
Learnt from: lucanicoladebiasi
PR: vechain/vechain-sdk-js#1777
File: packages/errors/src/available-errors/transaction/transaction.ts:33-33
Timestamp: 2025-01-30T17:15:49.377Z
Learning: PR #1777 is part of a phased approach to rename "delegator" terminology to "gas-payer" across the VeChain SDK. The changes are intentionally partial, with remaining occurrences to be addressed in subsequent PRs.
packages/core/src/transaction/Transaction.ts (1)
Learnt from: lucanicoladebiasi
PR: vechain/vechain-sdk-js#1777
File: packages/errors/src/available-errors/transaction/transaction.ts:33-33
Timestamp: 2025-01-30T17:15:49.377Z
Learning: PR #1777 is part of a phased approach to rename "delegator" terminology to "gas-payer" across the VeChain SDK. The changes are intentionally partial, with remaining occurrences to be addressed in subsequent PRs.
docs/transactions.md (1)
Learnt from: lucanicoladebiasi
PR: vechain/vechain-sdk-js#1777
File: packages/errors/src/available-errors/transaction/transaction.ts:33-33
Timestamp: 2025-01-30T17:15:49.377Z
Learning: PR #1777 is part of a phased approach to rename "delegator" terminology to "gas-payer" across the VeChain SDK. The changes are intentionally partial, with remaining occurrences to be addressed in subsequent PRs.
🪛 Biome (1.9.4)
apps/sdk-vite-integration/tsconfig.app.json
[error] 9-9: JSON standard does not allow comments.
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
apps/sdk-vite-integration/tsconfig.node.json
[error] 8-8: JSON standard does not allow comments.
(parse)
[error] 9-9: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 9-9: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 9-9: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 9-9: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
packages/core/src/transaction/Clause.ts
[error] 236-236: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
🪛 ESLint
docs/examples/contracts/contract-clauses.ts
[error] 21-21: 'transferVTHOClause' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
apps/sdk-vite-integration/src/components/GetLastBlock.tsx
[error] 1-1: Replace "@vechain/sdk-network"
with '@vechain/sdk-network'
(prettier/prettier)
[error] 2-2: Replace "react"
with 'react'
(prettier/prettier)
[error] 26-26: Replace {JSON.stringify(block,·null,·2)}
with ⏎························{JSON.stringify(block,·null,·2)}⏎····················
(prettier/prettier)
apps/sdk-vite-integration/src/components/Hash.tsx
[error] 3-3: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
apps/sdk-vite-integration/src/components/TransferLogs.tsx
[error] 8-8: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
[error] 8-8: Replace "../types/index.tsx"
with '../types/index.tsx'
(prettier/prettier)
[error] 9-9: Replace "../const/index.tsx"
with '../const/index.tsx'
(prettier/prettier)
apps/sdk-vite-integration/src/main.tsx
[error] 4-4: Insert ;
(prettier/prettier)
apps/sdk-vite-integration/tests/Hash.spec.tsx
[error] 3-3: Insert ;
(prettier/prettier)
[error] 6-6: Replace ··const·{·getByText·}·=·render(<Hash·/>)
with ····const·{·getByText·}·=·render(<Hash·/>);
(prettier/prettier)
[error] 8-8: Replace ··await·expect.element(getByText('0xbf56c0728fd4e9cf64bfaf6dabab81554103298cdee5cc4d580433aa25e98b00')).toBeInTheDocument()
with ····await·expect⏎········.element(⏎············getByText(⏎················'0xbf56c0728fd4e9cf64bfaf6dabab81554103298cdee5cc4d580433aa25e98b00'⏎············)⏎········)⏎········.toBeInTheDocument();
(prettier/prettier)
[error] 9-9: Insert ;
(prettier/prettier)
🪛 YAMLlint (1.35.1)
.github/workflows/gitleaks.yml
[warning] 5-5: wrong indentation: expected 10 but found 8
(indentation)
🔇 Additional comments (125)
packages/network/src/provider/utils/rpc-mapper/methods/eth_getTransactionByHash/eth_getTransactionByHash.ts (2)
11-11
: LGTM! Improved efficiency by using direct block hash lookup.The change to use
ethGetBlockByHash
is more efficient as it allows direct block lookup using the transaction'sblockID
instead of searching by block number.
45-48
: LGTM! Correct parameter usage for block retrieval.The block retrieval is properly implemented with appropriate parameters:
- Using
blockID
from transaction metadatafalse
parameter correctly indicates we don't need full transaction objectspackages/network/src/provider/utils/rpc-mapper/methods/eth_getStorageAt/eth_getStorageAt.ts (3)
1-1
: LGTM! Import changes align with standardization efforts.The transition to using
DefaultBlock
andDefaultBlockToRevision
aligns with the broader standardization of block parameter handling across RPC methods.Also applies to: 3-3
43-48
: LGTM! Parameter type change maintains type safety.The transition from
BlockQuantityInputRPC
toDefaultBlock
maintains type safety while standardizing block parameter handling across the codebase.
50-57
:❓ Verification inconclusive
Verify supported block numbers in DefaultBlockToRevision.
The change to use
DefaultBlockToRevision
aligns with the standardized approach. However, let's verify that it maintains support for only 'latest' and 'finalized' block numbers as documented.
🏁 Script executed:
#!/bin/bash # Search for DefaultBlockToRevision implementation to verify supported block numbers ast-grep --pattern 'export const DefaultBlockToRevision = $$$'Length of output: 64
Verify
DefaultBlockToRevision
support for correct block numbers.The change in using
DefaultBlockToRevision
appears consistent with the standardized block parameter approach. However, the automated search for its implementation did not return any output. Please manually verify that theDefaultBlockToRevision
implementation is limited to handling only the supported block numbers ('latest' and 'finalized'), as described in the JSDoc.
- Ensure that
DefaultBlockToRevision
enforces support for only 'latest' and 'finalized'.- Confirm that the standardized block parameter handling maintains the expected behavior in this context.
packages/aws-kms-adapter/tests/KMSVeChainSigner.solo.test.ts (2)
306-306
: LGTM! Improved signature validation.The regex pattern provides a more robust check by validating both the length and format of the signature, ensuring it's a valid 65-byte hex string with '0x' prefix.
308-322
:❓ Verification inconclusive
LGTM! Good test coverage and documentation.
The new test case improves coverage by verifying signing without primary type. The comments about LocalStack provide valuable context about signature variability.
Consider adding a link to the test implementation that confirms hash matching, mentioned in the comment "tests in the other implementation". This would help other developers locate the reference implementation.
🏁 Script executed:
#!/bin/bash # Search for related test implementations that verify hash matching rg -l "signTypedData.*hash.*match" --type tsLength of output: 46
LGTM! Test coverage and documentation look solid.
The new test case effectively verifies signing without a primary type, and the accompanying comments clarify the LocalStack issue well. However, the suggestion to add a link to the test implementation that confirms the hash matching couldn’t be automatically verified. Please manually double-check if a relevant test implementation exists and, if so, consider linking it to improve traceability.
packages/network/src/provider/utils/rpc-mapper/methods/eth_getBlockByHash/eth_getBlockByHash.ts (3)
8-10
: Imports look good.The new imports cleanly integrate the blocks formatter and the chain ID utility function. No further issues noted here.
22-22
: Documentation note acknowledged.The clarification that the Ethereum block hash is used as the Thor block ID is helpful.
52-58
: Conditional logic and block retrieval are well-structured.The function checks the transaction detail flag, then retrieves either an expanded or compressed block, returning
null
if none is found. This approach looks consistent with expected RPC behavior.apps/sdk-vite-integration/tsconfig.app.json (1)
6-6
:❓ Verification inconclusive
Verify compatibility with Vite bundling.
The change from ESNext/Bundler to Node16 for both
module
andmoduleResolution
might affect Vite's bundling capabilities, as Vite typically works best with ESM modules.Please verify:
- That these changes don't break Vite's build process
- That all imports (both internal and external) work as expected
- That tree-shaking is still effective
Also applies to: 10-10
🌐 Web query:
What are the recommended TypeScript compiler options for Vite projects?
💡 Result:
When configuring TypeScript for Vite projects, these compiler options are essential for optimal performance and compatibility:
Core Compiler Options
tsconfig.json
base configuration:{ "compilerOptions": { "target": "ES2020", "module": "ESNext", "moduleResolution": "bundler", "strict": true, "isolatedModules": true, "useDefineForClassFields": true, "skipLibCheck": true", "types": ["vite/client"] } }Key requirements:
isolatedModules: true
Mandatory for esbuild compatibility (Vite's transpiler)[7]. Disables features likeconst enum
that require full program analysis[7].
useDefineForClassFields: true
Required for modern class field behavior matching ES standards[7]. Automatically set forES2022+
targets[7].
moduleResolution: "bundler"
Enables Node.js-style resolution while supportingexports
conditions inpackage.json
[1][4].Strict type-checking flags:
{ "noUnusedLocals": true, "noUnusedParameters": true, "noFallthroughCasesInSwitch": true }Recommended for catching common errors during development[1][6].
Environment-Specific Configs
For monorepos or complex projects:
touch tsconfig.app.json tsconfig.node.jsonBrowser environment (
tsconfig.app.json
):{ "extends": "./tsconfig.json", "compilerOptions": { "lib": ["ES2020", "DOM", "DOM.Iterable"], "jsx": "react-jsx" }, "include": ["src"] }Node environment (
tsconfig.node.json
):{ "extends": "./tsconfig.json", "compilerOptions": { "target": "ES2022", "lib": ["ES2023"] }, "include": ["vite.config.ts"] }Additional Recommendations
- Type checking: Run
tsc --noEmit
separately as Vite doesn't perform type validation[3][7]- Path aliases: Use
vite-tsconfig-paths
plugin to resolve absolute imports[5][10]- Client types: Include
vite/client
for asset imports and env variables[7]- ESLint integration: Combine with
@typescript-eslint
plugins for code quality[6]For React projects using Vite's official template:
npm create vite@latest my-app -- --template react-tsThis pre-configures optimal TS settings[6][8].
Citations:
- 1: https://www.robinwieruch.de/vite-typescript/
- 2: https://v2.vitejs.dev/config/
- 3: https://v2.vitejs.dev/guide/features
- 4: https://vite.dev/config/
- 5: https://dev.to/michaeljota/how-to-add-jest-into-your-vite-project-with-ts-5d21
- 6: https://dev.to/oppaaaii/best-practices-for-using-typescript-in-react-with-vite-1dhf
- 7: https://vite.dev/guide/features
- 8: How to add typescript to an existing vite react app? vitejs/vite#6799
- 9: Ignoring all Typescript errors on build vitejs/vite#6716
- 10: https://www.npmjs.com/package/vite-tsconfig-paths
Attention: Verify Vite Bundling Compatibility
- The switch to
"module": "Node16"
(and, by implication,"moduleResolution": "Node16"
) departs from the recommended Vite configuration, which typically uses"module": "ESNext"
and"moduleResolution": "bundler"
.- This change may interfere with Vite's optimal functioning, impacting module import resolution and tree-shaking.
- Please confirm that:
- Vite’s build process remains unaffected.
- All module imports (both internal and external) resolve correctly.
- Tree-shaking continues to work as expected.
If this change was intentional, please ensure thorough testing to guarantee that Vite integration is not compromised.
packages/core/tests/vcdm/Revision.unit.test.ts (2)
34-34
: LGTM! Improved test description clarity.The updated test description better reflects the validation logic by using the more generic term "block tag" instead of listing specific values.
57-59
:❓ Verification inconclusive
LGTM! Added test coverage for 'next' block tag.
The new test case follows the established pattern and complements the existing test coverage for block tags.
Let's verify that the
Revision
class implementation supports the 'next' block tag:
🏁 Script executed:
#!/bin/bash # Search for the Revision class implementation to verify 'next' block tag support ast-grep --pattern 'class Revision { $$$ static isValid($_) { $$$ } $$$ }'Length of output: 88
Action Required: Verify Revision Implementation for 'next' Block Tag
The test case addition for the 'next' value looks good and follows our established pattern. However, our automated search using the providedast-grep
command didn't return any output, which suggests that either the pattern doesn’t match the current structure of theRevision
class or the implementation isn’t where we expect it to be. Please manually verify that theRevision.isValid('next')
call is handled correctly in the implementation (likely in a file such aspackages/core/src/vcdm/Revision.ts
or a similarly named file).
- Ensure that the
Revision
class properly supports the 'next' block tag (i.e., that it returns a truthy value for the input'next'
).- If the implementation has shifted in structure or location, update the automated checks accordingly.
apps/sdk-vite-integration/tsconfig.node.json (2)
5-5
: LGTM! Good choice of module settings.The change to "Node16" for both
module
andmoduleResolution
is appropriate for Node.js environments and aligns with modern ECMAScript modules support.Also applies to: 9-9
3-4
:❓ Verification inconclusive
Consider aligning target and lib versions.
The configuration uses ES2022 as the target but includes ES2023 library features. This might lead to runtime issues if ES2023 features are used but not supported in the ES2022 runtime environment.
Let's verify the ES2023 features compatibility:
🌐 Web query:
What ES2023 features are not supported in ES2022?
💡 Result:
ES2023 introduced several significant features not available in ES2022, focusing on improved array manipulation, syntax enhancements, and expanded data structure capabilities:
New Array Methods
findLast()
/findLastIndex()
: Retrieve the last element/index meeting a condition without reversing arrays[1][2][9].- Immutable variants: Non-mutating methods like
toSorted()
,toSpliced()
,toReversed()
, andwith()
return modified copies instead of altering original arrays[1][7][9].Example:
const arr = [5, 4, 3]; const sorted = arr.toSorted(); // Returns [3,4,5], original unchanged[1][9]Symbols as WeakMap Keys
Symbols can now serve as keys in WeakMaps (previously limited to objects), enabling memory-safe associations with unique identifiers[1][9][10].Hashbang Grammar Standardization
Directly specify interpreters (e.g.,#!/usr/bin/env node
) at script starts for cross-environment execution[1][2][9].Enhanced Array Modification
Thewith()
method safely replaces elements by index without mutating the original array[9]:const updated = arr.with(1, 99); // New array with index 1 set to 99[9]These additions address common pain points like mutating operations and backward iteration while aligning JavaScript closer to modern programming paradigms like immutability[1][7][9]. Browser support for these features is growing but still partial as of early 2025[2][10].
Citations:
- 1: https://www.telerik.com/blogs/exploring-latest-features-coming-javascript-es2023
- 2: https://daily.dev/blog/javascript-latest-version-an-overview
- 3: https://gist.github.com/Julien-Marcou/156b19aea4704e1d2f48adafc6e2acbf
- 4: https://eslint.org/docs/latest/use/configure/language-options-deprecated
- 5: https://javascript.plainenglish.io/javascript-reinvented-a-deep-dive-into-es2022-and-es2023-updates-cdb51853dd53
- 6:
ES2023
missing fortarget
in docs? microsoft/TypeScript#56303- 7: https://dev.to/jasmin/what-is-new-in-es2023-4bcm
- 8: https://www.typescriptlang.org/tsconfig/lib.html
- 9: https://codedamn.com/news/javascript/es2023-new-features
- 10: https://dev.to/jasmin/what-is-new-in-es2023-4bcm/comments
ES2023 lib vs. ES2022 target mismatch – review your configuration
The current configuration in
apps/sdk-vite-integration/tsconfig.node.json
sets"target": "ES2022"
while specifying"lib": ["ES2023"]
. This means that while type-checking recognizes ES2023 features (such as new array methods likefindLast()
,toSorted()
, enhancedwith()
operations, Symbols as WeakMap keys, and even hashbang syntax), the transpiled output will only conform to ES2022, potentially leading to runtime issues if those ES2023 features are actually used.
- Potential runtime pitfalls: ES2023 features (e.g.,
findLast()
,toSorted()
, and others) are not guaranteed to be supported in ES2022 environments.- Recommendation: Either align the target and library versions or ensure that any ES2023-specific features are appropriately polyfilled or transpiled to match the target runtime capabilities.
apps/sdk-vite-integration/src/components/GetLastBlock.tsx (3)
1-1
: LGTM! Good refactoring of thorClient import.Moving the thorClient instance to a centralized location improves maintainability and configuration management.
Also applies to: 3-3
🧰 Tools
🪛 ESLint
[error] 1-1: Replace
"@vechain/sdk-network"
with'@vechain/sdk-network'
(prettier/prettier)
20-22
: LGTM! Good addition of test identifier.Adding data-testid attribute improves component testability.
26-26
: LGTM! Good addition of test identifier.Adding data-testid attribute improves component testability.
🧰 Tools
🪛 ESLint
[error] 26-26: Replace
{JSON.stringify(block,·null,·2)}
with⏎························{JSON.stringify(block,·null,·2)}⏎····················
(prettier/prettier)
apps/sdk-vite-integration/src/components/TransferLogs.tsx (1)
1-184
: LGTM! Well-structured component with proper error handling.The component is well-implemented with:
- Clear separation of concerns
- Proper TypeScript types
- Good error handling
- Comprehensive UI feedback
🧰 Tools
🪛 ESLint
[error] 1-1: Replace
"react"
with'react'
(prettier/prettier)
[error] 3-6: All imports in the declaration are only used as types. Use
import type
.(@typescript-eslint/consistent-type-imports)
[error] 8-8: All imports in the declaration are only used as types. Use
import type
.(@typescript-eslint/consistent-type-imports)
[error] 8-8: Replace
"../types/index.tsx"
with'../types/index.tsx'
(prettier/prettier)
[error] 9-9: Replace
"../const/index.tsx"
with'../const/index.tsx'
(prettier/prettier)
[error] 25-25: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 115-115: Do not use Array index in keys
(sonarjs/no-array-index-key)
[error] 181-181: Insert
;
(prettier/prettier)
packages/network/src/provider/utils/const/blocks/blocks.ts (4)
4-17
: Type definition and block tags are well-structured.The union type
DefaultBlock
and thedefaultBlockTags
array cover all supported block tags consistently, aligning with the documented possibilities.
32-35
: Hex string handling looks correct.Converting the valid hex string to a numeric block number via
HexUInt
andRevision
appears consistent with the intended behavior.
38-46
: Good error handling for unknown default block tags.Throwing
JSONRPCInvalidDefaultBlock
when the block tag is not recognized provides clear feedback.
48-55
: Verify mapping of 'pending' to 'best'.Currently,
'pending'
also maps to'best'
. Confirm that'pending'
should indeed share the same revision as'latest'
instead of mapping to something like'next'
.Would you like to verify usage in the codebase or documentation references to confirm that
'pending'
→'best'
is correct?packages/core/src/vcdm/Revision.ts (5)
6-14
: Documentation updates are consistent.The revised doc comments clearly list new valid revision strings and provide clarity for permitted values like
'best', 'finalized', 'next', 'justified', 0x..., or positive integer
.
23-24
: Regex update accommodates newly allowed revision strings.The expanded pattern correctly encompasses “0x” prefixed hex, positive decimals, and recognized keywords like “best” and “finalized.”
27-28
: Parameter type acceptance is improved.Allowing
bigint
alongsidenumber
,string
, andHex
expands use cases without breaking existing logic.
31-41
: Validation logic handles multiple data types thoroughly.The approach for verifying
bigint
,number
,string
, andHex
is robust. The fallback toVALID_REVISION_REGEX
is consistent with the doc.
96-105
: Adding predefined static revisions is a nice convenience.
Revision.NEXT
andRevision.JUSTIFIED
improve readability and reduce magic strings.packages/core/src/transaction/Transaction.ts (8)
172-179
: Renamed accessors align with partial "delegator" → "gasPayer" rename.The doc block states the gas payer’s role clearly. This change is consistent with the phased approach to rename “delegator” references.
Also applies to: 175-176
283-284
: Clarifying delegated transaction state.Adding the remark that a delegated transaction signed only by the sender is still considered “not fully signed” is helpful.
296-298
: Doc updates for origin retrieval are consistent.Switching references from “delegator” to “gasPayer” elsewhere preserves clarity for the “origin” field.
380-387
: Optional gas payer parameter is logical.The
getTransactionHash(gasPayer?: Address)
approach clarifies that the gas payer’s signature depends on an extended hash.
499-501
: Valid checks prevent accidental delegated transaction sign calls.Throwing
InvalidTransactionField
if attempting to sign a delegated tx with.sign()
is a good safeguard to enforce correct usage.Also applies to: 511-513
554-557
: Appropriate constraints on gas payer signing.The guard clauses ensure the transaction is indeed delegated and partially signed by the sender, preventing incorrectly layered signatures.
Also applies to: 558-559, 561-561
590-590
: Accurate partial signing sequence.Using
.signAsSender
followed by.signAsGasPayer
is enforced clearly in the code flow withNotDelegatedTransaction
and validation checks.Also applies to: 592-594, 596-596, 605-607, 611-611
628-634
: Combined signing for sender and gas payer is well-structured.“signAsSenderAndGasPayer” consolidates the process, verifying both private keys and the delegated feature bit. Good approach for single-step usage.
Also applies to: 642-645, 647-663, 667-668, 673-682, 679-683
packages/core/tests/transaction/Transaction.unit.test.ts (25)
20-22
: All good here.
TheGasPayerPrivateKeyFix
fixture is well-defined for test use.
24-27
: Looks consistent with test fixture patterns.
DefiningGasPayerFix
consolidates the private key and derived address nicely.
159-161
: Test assertion ensures expected error behavior.
Verifying thatgasPayer
throwsNotDelegatedTransaction
for an undelegated transaction is correct.
185-187
: Appropriate error for unsigned delegated transaction.
ThrowingUnavailableTransactionField
forgasPayer
makes sense when it’s not yet established.
211-213
: Checks for delegated with unused fields.
Again, an unsigned delegated transaction justifiesUnavailableTransactionField
.
219-242
: Thorough partial-signing test scenario.
Simulating partial signatures between JS runtimes is a solid approach. The final multi-signature check is well done.
264-266
: Validates no gas payer in an undelegated + signed transaction.
Correctly throwsNotDelegatedTransaction
.
275-280
: Chaining transaction creation with signAsSenderAndGasPayer.
Implementation adheres to the new delegated signing logic.
294-294
: Verifies the final gas payer address.
This ensures the correct gas payer is recognized.
303-308
: Ensures signing works with “unused fields.”
This test confirms the transaction remains valid with additional reserved data.
322-322
: Gas payer address check is correct.
Accurately validates the second signer in a delegated transaction.
423-424
: Ensures gas payer is defined and matches.
This check confirms the second signature is properly attached.
473-474
: Same logic for delegated with unused fields.
Verifies the gas payer portion of the signature.
649-655
: Solid test for signing an undelegated transaction.
Confirms that a standard.sign()
call produces a valid single signature.
657-663
: Correctly disallows using.sign()
on delegated transactions.
ThrowsInvalidTransactionField
, preventing accidental misuse.
674-691
: New describe block covers signAsGasPayer behavior.
Extensive coverage for delegated vs. undelegated is beneficial.
693-702
: Disallows signAsGasPayer on undelegated transaction.
Matches theNotDelegatedTransaction
expectation.
704-713
: Throws for unsigned delegated tx when calling signAsGasPayer.
Ensures partial signing order is enforced.
715-723
: Verifies invalid private key usage for signAsGasPayer.
ThrowsInvalidSecp256k1PrivateKey
as expected.
726-726
: Additional describe block for signAsSender.
Expands coverage on partial-signed delegated transactions.
745-751
: Disallows signAsSender on undelegated tx.
Enforces correct method usage for delegated-only logic.
764-780
: Tests full signature flow for delegated transactions.
The combined signature (sender + gasPayer) approach is well validated.
782-791
: Ensures signAsSenderAndGasPayer throws on undelegated tx.
Maintains a strict boundary between delegated and standard transactions.
793-802
: Ensures invalid private keys are rejected (signer).
This test captures malformed keys for delegated sign.
804-813
: Ensures invalid private keys are rejected (gasPayer).
Validates the extra signature requirement for delegated transactions.packages/network/tests/provider/rpc-mapper/methods/eth_getBlockTransactionCountByNumber/fixture.ts (1)
15-15
: LGTM! Improved naming and organization.The renaming of test case variables and updated exports improve code clarity and maintainability.
Also applies to: 33-33
packages/errors/src/available-errors/transaction/transaction.ts (1)
32-34
: LGTM! Consistent terminology update.The change from
delegatorUrl
togasPayerUrl
aligns with the phased approach to rename "delegator" to "gas-payer" across the SDK.packages/core/src/vcdm/hash/Keccak256.ts (1)
6-8
: LGTM! Improved technical accuracy in documentation.The documentation now correctly refers to KECCAK 256 without mentioning SHA-3, which is more accurate as they are different algorithms.
Also applies to: 13-14
docs/examples/transactions/multiple-clauses.ts (1)
24-27
: LGTM! Method rename improves clarity.The change from
transferToken
totransferVTHOToken
makes the VTHO token transfer more explicit and self-documenting.packages/network/src/thor-client/contracts/types.d.ts (1)
30-44
: LGTM! Well-documented type extensions.The addition of
delegationUrl
andcomment
properties enhances the transaction options interface with clear documentation. The changes align with the broader shift in terminology from "delegator" to "gasPayer".packages/network/src/provider/utils/rpc-mapper/methods/eth_getCode/eth_getCode.ts (1)
41-49
: LGTM! Improved block parameter handling.The changes standardize block parameter handling by:
- Using more specific
DefaultBlock
type- Utilizing
DefaultBlockToRevision
for consistent block revision handlingpackages/network/src/provider/utils/rpc-mapper/methods/eth_getBalance/eth_getBalance.ts (1)
41-49
: LGTM! Consistent block parameter handling.The changes maintain consistency with other RPC methods by adopting the same block parameter handling approach using
DefaultBlock
andDefaultBlockToRevision
.packages/network/tests/provider/rpc-mapper/methods/eth_getBlockTransactionCountByNumber/eth_getBlockTransactionCountByNumber.testnet.test.ts (1)
8-8
: LGTM! The test case refactoring improves readability.The changes make the test cases more maintainable by:
- Using clearer fixture names (
validTestCases
,invalidTestCases
)- Using more descriptive variable names (
blockNumberHex
,expectedTxCount
)Also applies to: 36-38, 42-42, 45-45, 58-68
packages/network/tests/provider/rpc-mapper/methods/eth_getBlockByHash/fixture.ts (1)
40-41
: LGTM! The test case update improves clarity.The change adds a specific invalid block hash with a clear comment explaining its purpose for the testnet.
packages/errors/src/helpers/helpers.ts (1)
25-27
: LGTM! Good addition of bigint support.The implementation correctly handles bigint values by converting them to strings with the 'n' suffix, matching JavaScript's literal notation for bigint values.
docs/examples/thor-client/delegated-transactions.ts (1)
73-73
: LGTM! Improved method naming.The change from
signWithDelegator
tosignAsSenderAndGasPayer
makes the roles clearer and aligns with the updated terminology.packages/network/src/provider/utils/rpc-mapper/methods/eth_estimateGas/eth_estimateGas.ts (3)
33-33
: LGTM! Improved parameter validation.The validation now strictly requires exactly 2 parameters, making the function more robust.
44-47
: LGTM! Better type safety.The type changes from a generic unknown to specific types (
TransactionObjectInput
andDefaultBlock
) improve type safety.
48-48
: LGTM! Simplified block revision handling.The revision handling is now more streamlined using
DefaultBlockToRevision
.Also applies to: 60-61
packages/core/tests/vcdm/Address.unit.test.ts (2)
12-16
: LGTM! Improved test descriptions and organization.The test descriptions are now more descriptive and follow a consistent pattern (e.g., "ok <- valid lowercase"). The tests are well-organized and cover different address formats.
Also applies to: 18-22, 24-28, 30-35
37-40
: LGTM! Better error assertions.The error assertions now use the more modern
expect().toThrow()
syntax instead of try-catch blocks, making the tests cleaner and more maintainable.Also applies to: 54-59, 72-77
packages/errors/tests/helpers/helpers.unit.test.ts (1)
76-88
: LGTM! Test coverage for BigInt data type.The test case effectively verifies that the error message correctly handles and formats BigInt values.
docs/examples/transactions/full-flow-delegator-private-key.ts (1)
32-37
: LGTM! Consistent with new gas-payer terminology.Variable naming follows the new convention.
packages/errors/tests/available-errors/transaction.unit.test.ts (1)
67-99
: LGTM! Test cases updated for new gas-payer terminology.The test helper and test cases have been correctly updated to use the new
gasPayerUrl
parameter instead ofdelegatorUrl
.packages/errors/src/available-errors/provider/provider.ts (1)
154-155
: LGTM! Export list updated.The new error class is correctly added to the exports.
packages/network/tests/provider/rpc-mapper/methods/eth_call/fixture.ts (1)
64-73
: LGTM! Block number format aligns with Ethereum's JSON-RPC API.The change from using a block number object to a hex string representation follows Ethereum's JSON-RPC API specification for block parameters.
packages/hardhat-plugin/tests/helpers/fixture.ts (1)
41-44
: LGTM! Property renaming aligns with phased approach.The changes correctly rename only the inner properties (
delegatorPrivateKey
togasPayerPrivateKey
anddelegatorUrl
togasPayerServiceUrl
) while maintaining the parentdelegator
object, which aligns with the project's phased approach for BNC refactoring.Also applies to: 77-80, 119-122
packages/core/tests/keystore/keystore.unit.test.ts (3)
3-3
: LGTM!The addition of
InvalidDataType
error type enhances error handling capabilities.
64-68
: LGTM!The conditional error handling based on experimental cryptography flag improves error specificity.
145-149
: LGTM!The error handling for invalid keystore is now more precise based on experimental cryptography flag.
packages/core/src/vcdm/Address.ts (1)
88-93
: LGTM!Documentation improvements clarify the method's purpose and error handling.
packages/core/tests/secp256k1/Secp256k1.unit.test.ts (2)
10-38
: LGTM!Well-organized test data constants improve maintainability and readability.
88-101
: LGTM!Good addition of test case for hardware cryptography failure scenario.
packages/core/tests/hdkey/HDKey.unit.test.ts (2)
54-56
: LGTM! Improved test coverage with BIP44 paths.The added paths enhance test coverage by including BIP44 standard paths for Ethereum (60') and VeChain, which are essential for validating hierarchical deterministic key derivation.
69-70
: LGTM! Added invalid path test cases.The added invalid paths improve test coverage by verifying that paths not following the BIP44 standard are correctly rejected.
packages/network/src/thor-client/blocks/blocks-module.ts (1)
36-36
: LGTM! Parameter name improved for clarity.The constructor parameter has been renamed from
thor
tohttpClient
, which better describes its purpose and type.packages/network/src/signer/signers/types.d.ts (1)
162-170
: Well-documented interface extensions!The new optional properties
delegationUrl
andcomment
are clearly documented and enhance the transaction input capabilities.packages/network/src/signer/signers/vechain-private-key-signer/vechain-private-key-signer.ts (3)
74-78
: Documentation improvement!The updated JSDoc comment now correctly specifies that the method returns the address checksum rather than just the address.
106-115
: Good addition of delegation URL override!The code now allows for flexible gas payer configuration through either the wallet's delegator or a transaction-specific delegation URL.
247-336
: Consistent terminology update!The renaming from "delegator" to "gasPayer" improves code clarity and maintains consistency with the broader codebase changes.
packages/core/src/secp256k1/Secp256k1.ts (2)
135-159
: Security improvement in private key generation!The new implementation removes the potentially insecure AES-GCM fallback and provides better error handling with clear error messages.
228-228
: Enhanced parameter definition!Setting a default value of 32 for
bytesLength
improves usability while maintaining the common use case for cryptographic operations.packages/core/tests/transaction/Clause.unit.test.ts (1)
199-293
: Improved test specificity for VTHO operations!The renaming to
transferVTHOToken
and addition of transfer comments enhance test clarity and maintainability. The test suite maintains comprehensive coverage including error cases.packages/network/src/signer/signers/vechain-abstract-signer/vechain-abstract-signer.ts (2)
44-45
: LGTM!The comment update correctly reflects the new terminology.
187-190
: LGTM!The updated documentation provides a more accurate description of gas estimation for state changes.
packages/network/src/provider/utils/rpc-mapper/rpc-mapper.ts (1)
97-99
: LGTM!The new type definition improves code organization and type safety by:
- Consolidating related types in one place
- Explicitly defining generic parameters for better type inference
packages/network/src/thor-client/transactions/transactions-module.ts (1)
818-821
: LGTM!The changes consistently apply the new parameters across multiple transaction execution methods.
docs/tsconfig.json (1)
3-5
:✅ Verification successful
LGTM! Verify TypeScript version compatibility.
The changes to use ES2023 target and Node16 module system are good improvements that provide more stable and specific configurations. The changes are consistently applied across both sections.
Let's verify that the project's TypeScript version supports these options:
Also applies to: 11-13
🏁 Script executed:
#!/bin/bash # Description: Check TypeScript version and compatibility # Get TypeScript version from package.json jq -r '.dependencies.typescript // .devDependencies.typescript' package.json # Check if TypeScript is installed locally if [ -f "node_modules/.bin/tsc" ]; then # Get version from local installation ./node_modules/.bin/tsc --version fiLength of output: 170
Verified TypeScript Configuration Compatibility
The executed script confirms that the project's TypeScript version (5.6.3) supports the new configuration options (
"target": "ES2023"
,"module": "Node16"
,"moduleResolution": "Node16"
). The changes are consistently applied across the relevant sections (lines 3-5 and 11-13) indocs/tsconfig.json
.apps/sdk-vite-integration/tsconfig.json (1)
3-14
:❓ Verification inconclusive
LGTM! Verify Vitest types compatibility.
The configuration is well-structured for a Vite integration with appropriate TypeScript settings. The addition of
allowImportingTsExtensions
is correctly paired withnoEmit: true
.Let's verify the Vitest types compatibility:
🏁 Script executed:
#!/bin/bash # Description: Check Vitest types compatibility # Check if @vitest/browser package is installed and get its version jq -r '.dependencies."@vitest/browser" // .devDependencies."@vitest/browser"' package.json # Verify the types file exists fd -t f "playwright.d.ts" node_modules/@vitest/browser/providers/Length of output: 290
Rewritten Review Comment:
File: apps/sdk-vite-integration/tsconfig.json
Lines: 3–14The TypeScript configuration for the Vite integration looks solid, and the pairing of
allowImportingTsExtensions
withnoEmit: true
is correctly handled. However, please note that the"types": ["@vitest/browser/providers/playwright"]
entry still needs a manual check for compatibility. Our automated verification did not detect the@vitest/browser
dependency in package.json nor locate the expected types file in the presumed directory. Could you please confirm one of the following?
- That
@vitest/browser
(or its transitive provider) is installed and exposes theplaywright.d.ts
file.- That the current setup is intended to rely on a globally or transitively provided type declaration.
Once verified, we can ensure that Vitest types are indeed compatible as expected.
apps/sdk-cloudflare-integration/tsconfig.json (1)
4-6
:❓ Verification inconclusive
LGTM! Verify Cloudflare Workers types compatibility.
The configuration is well-suited for a Cloudflare Workers environment with appropriate TypeScript settings.
Let's verify the Cloudflare types compatibility:
Also applies to: 9-9
🏁 Script executed:
#!/bin/bash # Description: Check Cloudflare Workers types compatibility # Check if @cloudflare/workers-types package is installed and get its version jq -r '.dependencies."@cloudflare/workers-types" // .devDependencies."@cloudflare/workers-types"' package.json # Verify the experimental types file exists fd -t f "experimental.d.ts" node_modules/@cloudflare/workers-types/Length of output: 312
Attention: Manual Verification Required for Cloudflare Workers Types
The TypeScript configuration appears appropriate for a Cloudflare Workers environment. However, the automated checks for the
@cloudflare/workers-types
dependency produced no output:
- No version of
@cloudflare/workers-types
was found inpackage.json
(returnsnull
).- The expected
experimental.d.ts
file was not located in the directory.Please manually verify whether the Cloudflare Workers types are intentionally omitted (for example, relying on alternative global definitions) or if the dependency should be added to ensure proper type compatibility.
.github/workflows/on-main.yml (1)
8-10
: LGTM! Branch patterns expanded appropriately.The addition of
supported-*
,feature-*
, and1.0.0
branch patterns ensures proper CI coverage for feature development and version-specific branches..github/workflows/gitleaks.yml (1)
5-9
: LGTM! Branch patterns aligned with main workflow.The addition of branch patterns matches the changes in
on-main.yml
, ensuring consistent secret scanning across all relevant branches.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 5-5: wrong indentation: expected 10 but found 8
(indentation)
apps/sdk-nextjs-integration/tsconfig.json (1)
11-12
:✅ Verification successful
LGTM! Verify Node.js version compatibility.
The update to Node16 module system is appropriate for modern TypeScript projects.
🌐 Web query:
What is the minimum Node.js version required for Node16 module resolution in TypeScript?
💡 Result:
TypeScript's
"node16"
module resolution is designed to align with Node.js version 16 and later, which introduced stable ECMAScript Module (ESM) support alongside CommonJS[1][4][10]. This mode ensures TypeScript accurately models how Node.js resolves modules in ESM files (e.g., requiring file extensions like.js
in imports)[6][10].Key points:
- Node.js v12 added experimental ESM support[4][6], but
"node16"
specifically targets v16+ for full compatibility with stable ESM features likepackage.json
"exports"
/"imports"
[3][7].- Using
"moduleResolution": "node16"
requires pairing it with"module": "node16"
(or"nodenext"
) in yourtsconfig.json
[2][7].- Earlier versions of Node.js (pre-v16) may work with
"node16"
resolution if they support ESM syntax, but behavior isn't guaranteed[4][6].For projects targeting older Node.js versions (≤v10), use
"moduleResolution": "node10"
instead[4].Citations:
- 1: https://github.com/IanVS/ts-module-resolution-examples
- 2: https://www.typescriptlang.org/docs/handbook/modules/guides/choosing-compiler-options.html
- 3: https://matteosonoio.it/minimal-nodejs-typescript-setup/
- 4: https://www.typescriptlang.org/tsconfig/moduleResolution.html
- 5: Support TypeScript
"moduleResolution": "node16"
sveltejs/language-tools#1522- 6: https://devblogs.microsoft.com/typescript/announcing-typescript-5-0/
- 7: https://www.totaltypescript.com/concepts/option-module-must-be-set-to-nodenext-when-option-moduleresolution-is-set-to-nodenext
- 8: Support for TypeScript moduleResolution: "node16" molefrog/wouter#283
- 9: Support
Node16/NodeNext
value formoduleResolution
kulshekhar/ts-jest#4198- 10: https://www.youtube.com/watch?v=MEl2R7mEAP8
Ensure Node.js v16 or Later for Module Resolution
Your updated
tsconfig.json
configuration with:"module": "Node16", "moduleResolution": "Node16",is correct for modern TypeScript projects—as
"node16"
module resolution specifically requires Node.js version 16 or later to fully support stable ECMAScript modules. Please double-check that your development and deployment environments are running Node.js v16+ to ensure expected behavior.apps/sdk-vite-integration/package.json (2)
4-4
: LGTM! Version updates are consistent.The version bump to 1.0.0-rc.6 and corresponding dependency updates maintain consistency across the SDK.
Also applies to: 15-16
22-22
:✅ Verification successful
Verify compatibility of updated dev dependencies.
Please ensure that the updated versions of dev dependencies are compatible with the project:
- @eslint/js: 9.13.0 → 9.18.0
- @vitest/browser: 2.1.5 → 2.1.8
- vitest-browser-react: 0.0.3 → 0.0.4
Also applies to: 26-26, 36-36
🌐 Web query:
Are there any breaking changes in @eslint/js between versions 9.13.0 and 9.18.0?
💡 Result:
Between ESLint versions 9.13.0 and 9.18.0 (within the same major v9.x series), no official breaking changes were introduced according to the semantic versioning policy[7] and release notes[4][8][10]. However, there are nuanced considerations:
Key Points
- Semantic Versioning Policy: Minor releases (e.g., 9.x) avoid breaking API/config compatibility[7]. Breaking changes are reserved for major releases (e.g., v10).
- Release Notes Analysis:
- Versions like v9.14, v9.15, v9.16, v9.17, and v9.18 added features/bug fixes without backward-incompatible modifications[4][8][10].
- Example additions:
- Support for Import Attributes/RegExp Modifiers (v9.14)[4]
- New
meta.defaultOptions
field (v9.15)[4]- Performance optimizations (v9.x)[10]
Potential Impacts
While no explicit breaks exist:
1️⃣ Bug Fixes might cause new lint errors (e.g., stricter rule enforcement)[7].
2️⃣ New Features likemeta.defaultOptions
could affect custom plugins if not forward-compatible[4].
3️⃣ Dependency Updates (e.g.,@eslint/js@9.x
) may require syncing configurations[1].Recommended Action
- Review your project’s lint output after upgrading for new warnings/errors.
- Check plugin compatibility if using custom rules or integrations[1][2].
ESLint’s minor updates prioritize stability while enhancing functionality incrementally[7][10].
Citations:
- 1: https://eslint.org/docs/latest/use/migrate-to-9.0.0
- 2: https://vueschool.io/articles/vuejs-tutorials/upgrading-eslint-from-v8-to-v9-in-vue-js/
- 3: https://www.npmjs.com/package/eslint
- 4: https://github.com/eslint/eslint/releases
- 5: https://npm.io/package/eslint
- 6: https://github.com/eslint/eslint/blob/main/CHANGELOG.md?plain=1
- 7: https://www.npmjs.com/package/eslint?activeTab=versions
- 8: https://eslint.org/blog/2024/10/eslint-v9.13.0-released/
- 9: https://github.com/eslint/eslint/blob/main/CHANGELOG.md
- 10: https://eslint.org/blog/2025/01/eslint-v9.18.0-released/
Dependency Compatibility Verified: Upgraded Dev Dependencies Are Safe
The web query confirms that no official breaking changes were introduced in
@eslint/js
between 9.13.0 and 9.18.0. Although minor releases like these may introduce stricter lint rules or new bug fixes (which could surface additional lint errors), the changes adhere to semantic versioning. Please verify your project’s lint output after updating and ensure that similar checks are applied for the updates on@vitest/browser
(2.1.5 → 2.1.8) andvitest-browser-react
(0.0.3 → 0.0.4).
- apps/sdk-vite-integration/package.json (Lines 22, 26, & 36): Updated dependency versions are confirmed as non-breaking based on the release notes.
- Action Item: Re-run linting and tests post-update to catch any newly enforced rules.
apps/sdk-nextjs-integration/package.json (2)
4-4
: LGTM! Version updates are consistent.The version bump to 1.0.0-rc.6 and corresponding dependency updates maintain consistency across the SDK.
Also applies to: 17-18
20-20
:✅ Verification successful
Verify security of pinned nanoid version.
The nanoid version is pinned to 3.3.8 in both dependencies and resolutions. Let's verify this version for any security concerns.
Also applies to: 43-45
🌐 Web query:
Are there any known security vulnerabilities in nanoid version 3.3.8?
💡 Result:
nanoid version 3.3.8 was specifically released to address a critical security vulnerability (CVE-2024-55565) present in earlier versions[1][2][7]. Here's a detailed breakdown:
Key Vulnerability Fixed in 3.3.8
CVE-2024-55565:
- Issue: Mishandling of non-integer values caused an infinite loop, leading to application hangs or crashes[1][2][7].
- Impact: Denial of Service (DoS) due to unresponsive applications[2][4].
- CVSS Score: 4.3 (Medium severity)[4].
- Affected Versions: All releases before 3.3.8 (for v3.x) and 5.0.9 (for v5.x)[1][2][7].
Remediation
- Fix: Upgrade to nanoid ≥3.3.8 or ≥5.0.9[1][2][5][8].
- Workaround: Validate inputs as integers before processing[2]:
if (typeof input !== 'number' || !Number.isInteger(input)) { throw new Error('Input must be an integer'); }Verification
No additional vulnerabilities have been reported for nanoid@3.3.8 in the provided sources[9]. Projects like IBM App Connect Enterprise[4] and open-source repositories[5][8] confirmed upgrading to this version resolved CVE-2024-55565.
Summary
nanoid@3.3.8 is secure against CVE-2024-55565 and has no known vulnerabilities based on current disclosures[1][2][7][9]. Always monitor updates via tools like Vulert or dependabot for new findings[2][10].
Citations:
- 1: https://vuldb.com/?id.287293
- 2: https://vulert.com/vuln-db/CVE-2024-55565
- 3: https://secure.software/npm/packages/nanoid/issues/3.3.8
- 4: https://www.ibm.com/support/pages/security-bulletin-ibm-app-connect-enterprise-vulnerable-non-integer-values-mishandles-due-nanoid-cve-2024-55565
- 5: https://git.nicco.io/cupcakearmy/memoir/commit/eaac32317fa1e3a1240e724cc412b09987fc8adc
- 6: https://github.com/ai/nanoid
- 7: GHSA-mwcw-c2x4-8c55
- 8: https://git.fremnet.net/spacebar/client/commit/bbfc12c7a1f9f31b2625e04cf95b89395e4d70f9
- 9: https://npmjs.com/package/nanoid/v/3.3.8
- 10: Update dependencies so nanoid can be updated to 3.3.8 netlify/cli#6972
Security verification complete for nanoid@3.3.8
After reviewing external sources, we've confirmed that version 3.3.8 was released specifically to address CVE-2024-55565 and no further vulnerabilities are reported. The pinned version in the file (and its equivalent entry on lines 43-45) is secure and up-to-date.
- File: apps/sdk-nextjs-integration/package.json (Lines 20 and 43-45)
- Diff Snippet:
"nanoid": "3.3.8",No changes are needed regarding nanoid version security.
.github/workflows/on-pr.yml (1)
6-10
: LGTM! Branch patterns expanded appropriately.The workflow now covers additional branch patterns, improving CI coverage for different types of branches.
packages/core/package.json (1)
46-46
:✅ Verification successful
Verify compatibility with @scure/bip39 v1.5.1.
Let's check if there are any breaking changes in the new version.
🌐 Web query:
What are the breaking changes in @scure/bip39 between version 1.4.0 and 1.5.1?
💡 Result:
The provided search results do not contain explicit information about breaking changes between versions 1.4.0 and 1.5.1 of
@scure/bip39
. However, based on standard semantic versioning practices and analysis of available data:Key Observations:
- Security Focus: All versions of
@scure/bip39
emphasize audits and minimal dependencies[1][4][10].- No Breaking Changes Noted:
- No deprecations or API changes are documented in release notes or descriptions[4][6][7].
- Updates likely involved minor fixes or optimizations (e.g., dependency updates)[3][6].
Recommendations:
For precise details:
- Review GitHub Releases: Check paulmillr/scure-bip39 for explicit changelogs.
- Compare Source Code: Use
git diff 1.4.0..1.5.1
to inspect code changes directly.No breaking changes are evident from publicly available documentation in the provided sources[1][4][6][10].
Citations:
- 1: https://jsr.io/@scure/bip39
- 2: https://trezor.io/learn/a/what-is-bip39
- 3: https://classic.yarnpkg.com/en/package/viem
- 4: https://www.npmjs.com/package/@scure/bip39
- 5: https://github.com/paulmillr/scure-bip39/security
- 6: https://www.npmjs.com/package/@scure/bip39?activeTab=readme
- 7: https://www.npmjs.com/package/@metamask%2Fscure-bip39
- 8: https://bitcointalk.org/index.php?topic=5478242.20
- 9: https://secure.software/npm/packages/@scure/bip39/1.2.1
- 10: https://github.com/paulmillr/scure-bip39
Compatibility Verification: @scure/bip39 v1.5.1
- The update from previous versions to v1.5.1 does not show any documented breaking changes.
- Public release notes and semantic versioning practices suggest that the update involved minor fixes or optimizations.
- It is recommended to review the GitHub Releases for any future specifics, but currently the update appears safe.
packages/aws-kms-adapter/package.json (1)
41-41
:❓ Verification inconclusive
Verify AWS SDK KMS client compatibility.
The AWS SDK KMS client has been updated from ^3.696.0 to ^3.723.0. Let's verify the changes.
🌐 Web query:
What are the breaking changes in @aws-sdk/client-kms between version 3.696.0 and 3.723.0?
💡 Result:
Based on an analysis of available release notes and documentation for
@aws-sdk/client-kms
, here are the key breaking changes between versions 3.696.0 and 3.723.0:
1. Terminology Changes
- Deprecation of "CMK" terminology:
AWS replaced "customer master key (CMK)" with "KMS key" across APIs and documentation[6][10]. While backward compatibility was maintained for some variations:
- Code relying on deprecated parameter/response field names (e.g.,
CustomerMasterKeyId
) may break if these fields are removed or aliased inconsistently.- New features and documentation now exclusively use "KMS key"[6][10].
2. AWS Encryption SDK Integration Changes
- Key commitment enforcement:
Starting with AWS Encryption SDK v2.x (integrated into SDK updates), encryption/decryption requires explicit commitment policies[4][8]. This affects users leveraging KMS with encryption SDK features:
- Decryption fails for ciphertexts encrypted without key commitment (e.g., using AWS Encryption SDK v1.x)[4].
- Requires setting
CommitmentPolicy.FORBID_ENCRYPT_ALLOW_DECRYPT
to maintain backward compatibility[4].
3. AWS KMS Key Policy Behavior
- Strict mode enforcement:
AWS KMS key providers now default to strict mode, requiring explicit lists of wrapping keys for encryption/decryption[4][8]. Previously, some constructors allowed implicit discovery mode.
- Code using legacy discovery-mode constructors (deprecated in earlier versions) will break unless migrated to explicit strict/discovery modes[4][8].
Additional Notes
- No direct breaking changes were found in
@aws-sdk/client-kms
release notes for this version range[1][3]. The primary risks stem from dependencies like the AWS Encryption SDK and terminology shifts.- Always test upgrades in non-production environments to catch unannounced behavioral changes.
For detailed migration steps, refer to AWS’s official guide on Encryption SDK v2.x changes[5].
Citations:
- 1: https://github.com/aws/aws-sdk-js-v3/releases
- 2: https://github.com/aws/aws-sdk-ruby/blob/version-3/gems/aws-sdk-kms/CHANGELOG.md
- 3: https://github.com/aws/aws-sdk-js-v3/blob/main/packages/types/CHANGELOG.md
- 4: https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/about-versions.html
- 5: https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/migration.html
- 6: https://www.npmjs.com/package/@aws-sdk/client-kms?activeTab=versions
- 7: Amazon Connect Client break between version 3.696.0 and 3.699.0 aws/aws-sdk-js-v3#6695
- 8: https://aws-encryption-sdk-cli.readthedocs.io
- 9: https://security.snyk.io/package/npm/@aws-sdk%2Fclient-dynamodb
- 10: https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/client/kms
AWS SDK KMS Client Compatibility Verification
The dependency update to
"@aws-sdk/client-kms": "^3.723.0"
(updated from ^3.696.0) has been reviewed. Key points include:
- Terminology Update: AWS has shifted from using "CMK" to "KMS key". If your code depends on deprecated parameter or response field names (like
CustomerMasterKeyId
), ensure these are updated.- Encryption SDK Integration: The update aligns with changes in the AWS Encryption SDK (v2.x). This may require setting the appropriate key commitment policy (e.g.,
CommitmentPolicy.FORBID_ENCRYPT_ALLOW_DECRYPT
) to prevent decryption failures with legacy ciphertexts.- Strict Mode Enforcement: KMS key policy behavior now defaults to strict mode. Confirm that any legacy discovery-mode usage is updated to the proper strict configuration.
Please review these points and verify that any areas in your code using the AWS SDK KMS client have been updated accordingly, especially those interacting with encryption/decryption. Testing in a non-production environment is recommended to ensure compatibility.
docs/diagrams/architecture/transaction.md (1)
29-44
:❓ Verification inconclusive
Breaking change: Delegator renamed to Gas Payer.
The terminology change from "delegator" to "gasPayer" is a breaking change that affects the API. Ensure all documentation and client code is updated accordingly.
🏁 Script executed:
#!/bin/bash # Search for any remaining references to "delegator" in the codebase rg -i "delegator" --type ts --type js --type mdLength of output: 51473
Action Required: Confirm Transitional Naming in API Documentation
The diagram in
docs/diagrams/architecture/transaction.md
now shows the newgasPayer
field, but note that the function signature forgetTransactionHash(Address delegator?)
still uses the legacydelegator
parameter. Given that this PR is part of a phased approach (as noted in PR #1777) to eventually replace all occurrences of "delegator" with "gasPayer," the mix is expected for now. However, please ensure that:
- The transitional state (e.g., keeping
delegator
in certain function signatures) is clearly documented in the release notes or migration guide.- All client code and documentation updates explicitly communicate that the renaming is ongoing.
- A plan is in place to update remaining references in subsequent PRs, highlighting any potential backward compatibility implications.
packages/network/package.json (1)
45-47
: Version updates are consistent.All SDK dependencies are correctly aligned to version 1.0.0-rc.6.
docker/rpc-proxy/Dockerfile (2)
31-37
: Good security practice: OpenSSL upgrade.Explicitly upgrading OpenSSL and cleaning up cache is a good security practice.
22-25
:✅ Verification successful
Verify package adjustment script.
The new script modifies package.json files for production. Let's verify its functionality.
🏁 Script executed:
#!/bin/bash # Check if the adjust-packages.sh script exists and is executable ls -l ./docker/rpc-proxy/adjust-packages.sh # View the script contents cat ./docker/rpc-proxy/adjust-packages.shLength of output: 1950
Script verification passed – the package adjustment script is properly implemented.
- Verified that
./docker/rpc-proxy/adjust-packages.sh
exists with executable permissions.- The script correctly checks for a valid input directory and processes all
package.json
files (excludingnode_modules
).- It removes the
devDependencies
section and strips caret (^
) and tilde (~
) version selectors from dependencies as intended.package.json (1)
54-54
:❓ Verification inconclusive
Verify compatibility with eslint-config-prettier v10
The update to eslint-config-prettier from v9 to v10 is a major version bump that may include breaking changes.
Also applies to: 58-58, 62-62, 64-64, 79-79
🌐 Web query:
What are the breaking changes in eslint-config-prettier v10?
💡 Result:
The key breaking changes in eslint-config-prettier v10 involve integration with new formatting rules and plugin support:
Major Changes in v10
Support for @stylistic/eslint-plugin
- Added explicit handling of formatting rules from
@stylistic/eslint-plugin
(formerly@typescript-eslint/eslint-plugin
) [6][9][10]- Requires updates if using stylistic rules that conflict with Prettier's formatting
Notable Changes from Earlier Major Versions
While not part of v10 specifically, recent major versions introduced other breaking changes:
From v9 (2023):
- Dropped support for
eslint-plugin-typescript
in favor of@typescript-eslint/eslint-plugin
[1]- Stopped disabling
unicode-bom
rule by default (requires manual config adjustment) [1][2]From v8 (2021):
- Merged all plugin-specific configs into single
"prettier"
entry [1]- Simplified usage:
Instead of multiple{ "extends": ["prettier"] }prettier/<plugin>
entries [1]For most users upgrading to v10, the main impact comes from needing to handle
@stylistic
plugin rules if used in their ESLint configuration[6][9]. The CLI helper tool now automatically detects conflicts with these new rules when runningnpx eslint-config-prettier
[1][9].Citations:
- 1: https://github.com/prettier/eslint-config-prettier/blob/main/CHANGELOG.md
- 2: https://online-gymnasium.gitlab.yandexcloud.net/frontend-modules/biorich-header/-/merge_requests/8
- 3: https://eslint.org/docs/latest/use/configure/configuration-files
- 4: https://loopback.io/doc/en/lb4/changelog.eslint-config.html
- 5: https://gitlab.duraken.com/fitness-providers/fitness-providers/-/merge_requests/1825
- 6: https://github.com/prettier/eslint-config-prettier/releases
- 7: https://git.daniel-siepmann.de/danielsiepmann/tea/commit/e6023e380807ef922cffca94b8d00d47cedd8115
- 8: https://eslint.org/blog/2024/04/eslint-v9.0.0-released/
- 9: https://www.npmjs.com/package/eslint-config-prettier
- 10: https://newreleases.io/project/github/prettier/eslint-config-prettier/release/v10.0.1
Verify compatibility with eslint-config-prettier v10
The update from eslint-config-prettier v9.1.0 to v10.0.1 is a major version bump that introduces explicit handling for formatting rules from
@stylistic/eslint-plugin
. If your ESLint configuration uses stylistic rules or any related plugins, please ensure that:
- No conflicts arise with Prettier’s formatting rules due to the new integration.
- Any previously adjusted configurations (such as manual handling of the
unicode-bom
rule) are re‑evaluated in light of these changes.Also, remember to verify the impact in the referenced locations (package.json lines 54, 58, 62, 64, and 79).
docs/contracts.md (2)
40-43
: LGTM! Method name change improves clarityThe rename from
transferToken
totransferVTHOToken
makes the method's purpose more explicit.
205-205
: LGTM! Documentation update for terminology changeThe documentation correctly reflects the transition from "delegator" to "gas-payer" terminology, including a deprecation notice.
Also applies to: 212-213
docs/thor-client.md (1)
362-365
: LGTM! Method name change improves clarityThe rename from
signWithDelegator
tosignAsSenderAndGasPayer
better describes the roles involved in the signing process.docs/transactions.md (1)
466-467
: LGTM! Consistent terminology updatesThe examples have been updated to use the new "gas-payer" terminology consistently, with appropriate deprecation notices.
Also applies to: 487-490, 574-575, 593-596
@@ -1,7 +1,7 @@ | |||
import { StrictMode } from 'react' | |||
import { createRoot } from 'react-dom/client' | |||
import './index.css' | |||
import App from './App' | |||
import App from './App.tsx' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify build configuration for explicit file extensions.
While adding explicit file extensions improves clarity, ensure that your build configuration (Vite/TypeScript) is set up to handle these imports correctly.
Run this script to check the build configuration:
🏁 Script executed:
#!/bin/bash
# Description: Check TypeScript and Vite configuration for handling .tsx extensions
# Check TypeScript configuration
cat tsconfig*.json | jq '.compilerOptions | select(.moduleResolution != null)'
# Check Vite configuration
cat vite.config.* | grep -A 5 'resolve'
Length of output: 227
Below is a revised review comment that reflects the inconclusive output from the initial verification script. Since the original script did not return the expected results—indicating potential issues with parsing your tsconfig file and/or that a Vite configuration file wasn’t found—you’ll need to manually verify that your build configuration supports explicit .tsx
file imports.
Attention: Manual Verification Required for Build Configuration
It appears that the automated checks for your TypeScript and Vite settings did not return conclusive results:
- TypeScript Configuration: The script reported an error when parsing
tsconfig.json
(e.g., “parse error: Invalid numeric literal at line 23”). Please verify thattsconfig.json
is valid JSON and that thecompilerOptions
(especially settings likemoduleResolution
) are correctly defined. - Vite Configuration: No Vite config file (e.g.,
vite.config.js
orvite.config.ts
) was detected. Confirm that your Vite setup includes the appropriateresolve
settings to handle explicit file extensions.
Before merging, please ensure these configurations are set up properly and that the build process can successfully resolve imports with explicit file extensions.
If needed, you can run the following script to help locate and validate your configuration files:
Please let me know if any further investigation is required.
🏁 Script executed:
[run_scripts]
#!/bin/bash
echo "Validating tsconfig.json..."
if [ -f "tsconfig.json" ]; then
jq . tsconfig.json || echo "Error: tsconfig.json contains invalid JSON."
else
echo "tsconfig.json not found."
fi
echo "Searching for Vite configuration files..."
fd -HI '^vite\.config\..*$' || echo "Vite configuration file not found at the repository root."
Length of output: 553
Action Required: Fix Build Configuration Issues
- tsconfig.json Issue: The output indicates that your
tsconfig.json
contains invalid JSON (parse error at line 23, column 7). This error must be resolved, as an incorrect TypeScript configuration could disrupt how explicit.tsx
extensions are handled. - Vite Configuration: A Vite configuration file was found at
apps/sdk-vite-integration/vite.config.ts
. Please verify that itsresolve
settings are correctly set up to handle imports with explicit file extensions.
Before merging, ensure that the JSON in your tsconfig.json
is valid and that both build configurations are correctly processing explicit file extensions.
🧰 Tools
🪛 ESLint
[error] 4-4: Insert ;
(prettier/prettier)
@@ -1,9 +1,9 @@ | |||
import { expect, test } from 'vitest' | |||
import { render } from 'vitest-browser-react' | |||
import Hash from '../src/components/Hash' | |||
import Hash from '../src/components/Hash.js' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix inconsistent file extension in import path.
The import path uses .js
extension while the component file has .tsx
extension. This is inconsistent with other changes in the codebase and might cause issues.
Apply this diff to maintain consistency:
-import Hash from '../src/components/Hash.js'
+import Hash from '../src/components/Hash.tsx'
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import Hash from '../src/components/Hash.js' | |
import Hash from '../src/components/Hash.tsx' |
🧰 Tools
🪛 ESLint
[error] 3-3: Insert ;
(prettier/prettier)
test('Should throw `JSONRPCInvalidParams` if the default block parameter is invalid block tag', async () => { | ||
await expect( | ||
RPCMethodsMap(thorClient)[RPC_METHODS.eth_call]([ | ||
{ | ||
from: '0x7487d912d03ab9de786278f679592b3730bdd540', | ||
to: '0x3db469a79593dcc67f07DE1869d6682fC1eaf535', | ||
value: '1000000000000000000', | ||
data: '0x' | ||
}, | ||
'invalid' | ||
]) | ||
).rejects.toThrowError(JSONRPCInternalError); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix error expectation for invalid block tag.
The test expects JSONRPCInternalError
for invalid block tag, but according to the method's documentation, it should throw JSONRPCInvalidParams
.
Apply this diff to fix the error expectation:
- ).rejects.toThrowError(JSONRPCInternalError);
+ ).rejects.toThrowError(JSONRPCInvalidParams);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
test('Should throw `JSONRPCInvalidParams` if the default block parameter is invalid block tag', async () => { | |
await expect( | |
RPCMethodsMap(thorClient)[RPC_METHODS.eth_call]([ | |
{ | |
from: '0x7487d912d03ab9de786278f679592b3730bdd540', | |
to: '0x3db469a79593dcc67f07DE1869d6682fC1eaf535', | |
value: '1000000000000000000', | |
data: '0x' | |
}, | |
'invalid' | |
]) | |
).rejects.toThrowError(JSONRPCInternalError); | |
}); | |
test('Should throw `JSONRPCInvalidParams` if the default block parameter is invalid block tag', async () => { | |
await expect( | |
RPCMethodsMap(thorClient)[RPC_METHODS.eth_call]([ | |
{ | |
from: '0x7487d912d03ab9de786278f679592b3730bdd540', | |
to: '0x3db469a79593dcc67f07DE1869d6682fC1eaf535', | |
value: '1000000000000000000', | |
data: '0x' | |
}, | |
'invalid' | |
]) | |
).rejects.toThrowError(JSONRPCInvalidParams); | |
}); |
test('Should throw `JSONRPCInvalidParams` if the default block parameter is invalid block number hex', async () => { | ||
await expect( | ||
RPCMethodsMap(thorClient)[RPC_METHODS.eth_call]([ | ||
{ | ||
from: '0x7487d912d03ab9de786278f679592b3730bdd540', | ||
to: '0x3db469a79593dcc67f07DE1869d6682fC1eaf535', | ||
value: '1000000000000000000', | ||
data: '0x' | ||
}, | ||
'0xinvalid' | ||
]) | ||
).rejects.toThrowError(JSONRPCInternalError); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix error expectation for invalid block number hex.
Similar to the previous test, this test also expects JSONRPCInternalError
for invalid block number hex, but it should throw JSONRPCInvalidParams
.
Apply this diff to fix the error expectation:
- ).rejects.toThrowError(JSONRPCInternalError);
+ ).rejects.toThrowError(JSONRPCInvalidParams);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
test('Should throw `JSONRPCInvalidParams` if the default block parameter is invalid block number hex', async () => { | |
await expect( | |
RPCMethodsMap(thorClient)[RPC_METHODS.eth_call]([ | |
{ | |
from: '0x7487d912d03ab9de786278f679592b3730bdd540', | |
to: '0x3db469a79593dcc67f07DE1869d6682fC1eaf535', | |
value: '1000000000000000000', | |
data: '0x' | |
}, | |
'0xinvalid' | |
]) | |
).rejects.toThrowError(JSONRPCInternalError); | |
}); | |
test('Should throw `JSONRPCInvalidParams` if the default block parameter is invalid block number hex', async () => { | |
await expect( | |
RPCMethodsMap(thorClient)[RPC_METHODS.eth_call]([ | |
{ | |
from: '0x7487d912d03ab9de786278f679592b3730bdd540', | |
to: '0x3db469a79593dcc67f07DE1869d6682fC1eaf535', | |
value: '1000000000000000000', | |
data: '0x' | |
}, | |
'0xinvalid' | |
]) | |
).rejects.toThrowError(JSONRPCInvalidParams); | |
}); |
@@ -12,7 +12,7 @@ import { type HttpNetworkConfig } from 'hardhat/types'; | |||
* | |||
* They have custom parameters: | |||
* - debug: whether to enable debug mode | |||
* - delegator: the delegator to use | |||
* - gasPayer: the gasPayer to use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert parent object renaming to maintain phase consistency.
According to the project's phased approach for BNC refactoring, only inner properties should be renamed in this phase (e.g., delegatorPrivateKey
to gasPayerPrivateKey
), while the parent object delegator
should remain unchanged until a future phase.
Apply this diff to maintain phase consistency:
- gasPayer: undefined,
+ delegator: undefined,
- gasPayer: {
- gasPayerServiceUrl:
- 'https://sponsor-testnet.vechain.energy/by/269'
+ delegator: {
+ gasPayerServiceUrl:
+ 'https://sponsor-testnet.vechain.energy/by/269'
- gasPayer: {
- gasPayerPrivateKey:
+ delegator: {
+ gasPayerPrivateKey:
Also applies to: 45-45, 68-68, 91-93, 118-122, 140-141
public static transferVTHOToken( | ||
recipientAddress: Address, | ||
amount: VTHO, | ||
clauseOptions?: ClauseOptions | ||
amount: VTHO | ||
): Clause { | ||
if (amount.value.isFinite() && amount.value.isPositive()) { | ||
const vthoAddress = Address.of(VTHO_ADDRESS); | ||
return this.callFunction( | ||
tokenAddress, | ||
vthoAddress, | ||
ABIContract.ofAbi(VIP180_ABI).getFunction( | ||
Clause.TRANSFER_TOKEN_FUNCTION | ||
), | ||
[recipientAddress.toString(), amount.wei], | ||
undefined, | ||
clauseOptions | ||
{ comment: 'Transfer VTHO' } | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
LGTM! Method renamed for clarity and specificity.
The method has been appropriately renamed from transferToken
to transferVTHOToken
to better reflect its specific purpose. The removal of the tokenAddress
parameter and use of VTHO_ADDRESS
constant improves type safety and prevents errors.
Fix static method using this
.
Using this
in a static context can be confusing. Replace this.callFunction
with Clause.callFunction
for better clarity.
Apply this diff to fix the static context issue:
- return this.callFunction(
+ return Clause.callFunction(
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
public static transferVTHOToken( | |
recipientAddress: Address, | |
amount: VTHO, | |
clauseOptions?: ClauseOptions | |
amount: VTHO | |
): Clause { | |
if (amount.value.isFinite() && amount.value.isPositive()) { | |
const vthoAddress = Address.of(VTHO_ADDRESS); | |
return this.callFunction( | |
tokenAddress, | |
vthoAddress, | |
ABIContract.ofAbi(VIP180_ABI).getFunction( | |
Clause.TRANSFER_TOKEN_FUNCTION | |
), | |
[recipientAddress.toString(), amount.wei], | |
undefined, | |
clauseOptions | |
{ comment: 'Transfer VTHO' } | |
); | |
public static transferVTHOToken( | |
recipientAddress: Address, | |
amount: VTHO | |
): Clause { | |
if (amount.value.isFinite() && amount.value.isPositive()) { | |
const vthoAddress = Address.of(VTHO_ADDRESS); | |
return Clause.callFunction( | |
vthoAddress, | |
ABIContract.ofAbi(VIP180_ABI).getFunction( | |
Clause.TRANSFER_TOKEN_FUNCTION | |
), | |
[recipientAddress.toString(), amount.wei], | |
undefined, | |
{ comment: 'Transfer VTHO' } | |
); | |
} | |
} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 236-236: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
// If the block is successfully fetched (not undefined or null), update the result. | ||
if (block !== undefined && block !== null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistency between comment and code.
The comment has been updated to remove the mention of incrementing the block number, but the code at line 137 still increments this.subscriptionManager.currentBlockNumber
. This creates a discrepancy between the documentation and the actual behavior.
Either:
- Restore the comment to reflect the actual behavior:
- // If the block is successfully fetched (not undefined or null), update the result.
+ // If the block is successfully fetched (not undefined or null), update the result and increment the block number.
Or:
2. Remove the block number increment if it's no longer needed:
- this.subscriptionManager.currentBlockNumber++;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// If the block is successfully fetched (not undefined or null), update the result. | |
if (block !== undefined && block !== null) { | |
// If the block is successfully fetched (not undefined or null), update the result and increment the block number. | |
if (block !== undefined && block !== null) { |
// The term `delegator` will be deprecated soon and renamed `gasPayer`. | ||
delegator: { | ||
delegatorPrivateKey: delegatorAccount.privateKey | ||
delegatorPrivateKey: gasPayerAccount.privateKey | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Update configuration to use consistent terminology.
The configuration shows mixed usage of old and new terminology:
- Comment mentions "delegator will be deprecated"
- Configuration still uses
delegator
anddelegatorPrivateKey
Apply this diff to maintain consistent terminology:
- // The term `delegator` will be deprecated soon and renamed `gasPayer`.
- delegator: {
- delegatorPrivateKey: gasPayerAccount.privateKey
- }
+ gasPayer: {
+ gasPayerPrivateKey: gasPayerAccount.privateKey
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// The term `delegator` will be deprecated soon and renamed `gasPayer`. | |
delegator: { | |
delegatorPrivateKey: delegatorAccount.privateKey | |
delegatorPrivateKey: gasPayerAccount.privateKey | |
} | |
gasPayer: { | |
gasPayerPrivateKey: gasPayerAccount.privateKey | |
} |
#!/bin/bash | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for jq availability.
The script relies on jq but doesn't check if it's installed.
#!/bin/bash
+
+# Check if jq is available
+if ! command -v jq &> /dev/null; then
+ echo "Error: jq is required but not installed."
+ exit 1
+fi
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
#!/bin/bash | |
#!/bin/bash | |
# Check if jq is available | |
if ! command -v jq &> /dev/null; then | |
echo "Error: jq is required but not installed." | |
exit 1 | |
fi |
Description
BNC part 3 backport.
Summary by CodeRabbit
New Features
Refactor
Chore
Documentation
Tests