fix: smart contract klv tx #96
Conversation
WalkthroughThis pull request introduces new tests for transaction signing in both the Wallet and KLV modules, verifies the proper functioning of signature generation and transaction hashing, and improves data handling in the models by making the nonce field optional with default fallbacks. In addition, a new smart contract type is added to the protocol definition. Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test
participant W as Wallet Module
T->>W: Create wallet using mnemonic
T->>W: Sign transaction with raw JSON
W-->>T: Return signed transaction (signature & hash)
sequenceDiagram
participant T as Test
participant K as KLV Module
T->>K: Call sign_tx with smart contract JSON
K-->>T: Return transaction hash
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 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/kos/src/protos/klever/transaction.proto (1)
37-37: Add documentation for SmartContractType.While the addition of SmartContractType is good, please add documentation to describe:
- The purpose and use cases of this contract type
- Why the value 63 was chosen
- Any special handling requirements
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/kos/src/protos/generated/klv/proto.rsis excluded by!**/generated/**
📒 Files selected for processing (4)
packages/kos-web/src/wallet.rs(1 hunks)packages/kos/src/chains/klv/mod.rs(1 hunks)packages/kos/src/chains/klv/models.rs(2 hunks)packages/kos/src/protos/klever/transaction.proto(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: format
🔇 Additional comments (4)
packages/kos/src/chains/klv/models.rs (2)
36-36: LGTM! Making nonce optional improves flexibility.The change to make nonce optional with
Option<u64>is a good improvement as it allows for cases where nonce may not be provided.
154-154: LGTM! Proper handling of optional nonce.The implementation correctly handles the optional nonce by using
unwrap_or(0)to provide a default value when nonce is None.packages/kos/src/chains/klv/mod.rs (1)
288-311: LGTM! Good test coverage for smart contract transactions.The test case properly validates the transaction signing functionality for smart contracts by:
- Using a realistic transaction JSON
- Verifying the transaction hash
- Following the established test pattern
packages/kos-web/src/wallet.rs (1)
491-506: LGTM! Good integration test for wallet transaction signing.The test provides good coverage by:
- Testing the full wallet creation and transaction signing flow
- Verifying both signature and transaction hash generation
- Using a realistic smart contract transaction
Summary by CodeRabbit
New Features
Refactor
Tests