Fix SRC-01 SRO-01 #1770
Fix SRC-01 SRO-01 #1770
Conversation
WalkthroughThe pull request introduces a refined method for transferring VTHO tokens within the VeChain SDK. The primary change involves renaming Changes
Sequence DiagramsequenceDiagram
participant Sender
participant Clause
participant VTHO Contract
Sender->>Clause: transferVTHOToken(recipientAddress, amount)
Clause->>VTHO Contract: Execute Token Transfer
VTHO Contract-->>Sender: Transfer Confirmation
Possibly related PRs
Suggested Labels
Suggested Reviewers
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/src/transaction/Clause.ts (1)
230-243: Refactor static method to avoid 'this' usage.The static method uses
this.callFunctionwhich could be confusing. Consider using the class name explicitly.- return this.callFunction( + return Clause.callFunction(🧰 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)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/contracts.md(1 hunks)docs/examples/contracts/contract-clauses.ts(1 hunks)docs/examples/transactions/multiple-clauses.ts(1 hunks)docs/transactions.md(1 hunks)packages/core/src/transaction/Clause.ts(6 hunks)packages/core/tests/transaction/Clause.unit.test.ts(6 hunks)packages/network/src/provider/providers/hardhat-provider/hardhat-provider.ts(1 hunks)packages/network/src/signer/signers/vechain-abstract-signer/vechain-abstract-signer.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/network/src/provider/providers/hardhat-provider/hardhat-provider.ts
- packages/network/src/signer/signers/vechain-abstract-signer/vechain-abstract-signer.ts
🧰 Additional context used
🪛 ESLint
docs/examples/contracts/contract-clauses.ts
[error] 21-21: 'transferVTHOClause' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🪛 Biome (1.9.4)
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)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: rpc-proxy / test / test
- GitHub Check: unit-integration-test-browser / Build & Lint (latest)
- GitHub Check: unit-integration-test / Build & Lint (latest)
- GitHub Check: unit-integration-test-browser / Build & Lint (lts/*)
- GitHub Check: unit-integration-test / Build & Lint (lts/*)
- GitHub Check: unit-integration-test-browser / Build & Lint (18)
- GitHub Check: unit-integration-test / Build & Lint (18)
- GitHub Check: install-build / Build & Lint
- GitHub Check: test-apps / Install and test example apps
- GitHub Check: Execute doc examples
🔇 Additional comments (6)
docs/examples/contracts/contract-clauses.ts (1)
21-24: LGTM! Method name change improves clarity.The update from
transferTokentotransferVTHOTokenmakes the VTHO-specific nature of the transfer explicit.🧰 Tools
🪛 ESLint
[error] 21-21: 'transferVTHOClause' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
docs/examples/transactions/multiple-clauses.ts (1)
24-27: LGTM! Example updated correctly.The multiple clause example correctly demonstrates the new
transferVTHOTokenmethod alongside VET transfers.packages/core/src/transaction/Clause.ts (1)
Line range hint
219-250: LGTM! Method refactored for VTHO-specific transfers.The method has been simplified by removing the redundant
tokenAddressparameter and using the constantVTHO_ADDRESS. The error messages and comments have been updated accordingly.🧰 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)
packages/core/tests/transaction/Clause.unit.test.ts (1)
Line range hint
199-291: LGTM! Comprehensive test coverage for VTHO transfers.The test suite thoroughly covers:
- Various VTHO amounts (1 wei, 100 wei, 1 VTHO, 500M VTHO)
- Error cases (negative, infinite, NaN amounts)
- Correct encoding of transfer data
docs/contracts.md (1)
40-43: LGTM! Improved API clarity for VTHO token transfers.The renaming from
transferTokentotransferVTHOTokenmakes the API more explicit and self-documenting about the specific token being transferred.docs/transactions.md (1)
77-80: LGTM! Consistent method naming across documentation.The update to
transferVTHOTokenmaintains consistency with the changes in other files, ensuring a unified API for VTHO token transfers.
Test CoverageSummary
|
Description
Fix of audit items SRC-01 and SRO-01
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist:
Summary by CodeRabbit
Release Notes
Documentation
transferTokentotransferVTHOTokenBug Fixes
The changes improve clarity and specificity of token transfer methods in the VeChain SDK documentation and code.