Conversation
Implement TransactionAction to query transaction fee events from the ledger. Enables applications to fetch transaction details including fee amounts, recipients, and distribution breakdowns. resolves: trufnetwork/truf-network#1331
WalkthroughIntroduces TransactionAction class for querying transaction ledger data via Kwil client, with new type definitions (FeeDistribution, TransactionEvent, GetTransactionEventInput), a loadTransactionAction() method on BaseTNClient, integration tests, and utility functions for transaction ID normalization. Changes
Sequence DiagramsequenceDiagram
actor User
participant Client as BaseTNClient
participant TxAction as TransactionAction
participant Backend as Kwil Backend
User->>Client: loadTransactionAction()
Client->>TxAction: new TransactionAction(kwilClient, signer)
TxAction-->>Client: TransactionAction instance
Client-->>User: TransactionAction
User->>TxAction: getTransactionEvent({txId})
TxAction->>TxAction: validate txId (non-empty)
TxAction->>Backend: get_transaction_event(txId)
Backend-->>TxAction: transaction data
TxAction->>TxAction: validate fields (method, caller, fee_amount)
TxAction->>TxAction: parse fee_distributions (recipient:amount pairs)
TxAction->>TxAction: validate block_height (finite, non-negative)
TxAction->>TxAction: construct & normalize TransactionEvent
TxAction-->>User: TransactionEvent
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 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. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Time Submission Status
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/integration/transactionAction.test.ts (1)
70-72: Consider extracting duplicate tx hash normalization logic.The tx hash normalization logic appears twice (lines 70-72 and lines 124-126). Consider extracting this into a helper function to improve maintainability.
// In tests/integration/utils.ts or within this test file function normalizeTxHash(txHash: string): string { return txHash.startsWith("0x") ? txHash.toLowerCase() : `0x${txHash.toLowerCase()}`; }Then use it in the tests:
- const normalizedTxId = txEvent.txId.toLowerCase(); - const normalizedTxHash = txHash.startsWith("0x") ? txHash.toLowerCase() : `0x${txHash.toLowerCase()}`; - expect(normalizedTxId).toBe(normalizedTxHash); + expect(txEvent.txId.toLowerCase()).toBe(normalizeTxHash(txHash));Also applies to: 124-126
src/contracts-api/transactionAction.ts (2)
56-103: Consider improving type safety for the backend response.Using
as anyon line 56 loses all type safety. Consider defining an interface for the expected row shape to catch potential issues at development time.interface TransactionEventRow { tx_id: string; block_height: string; method: string; caller: string; fee_amount: string; fee_recipient: string | null; metadata: string | null; fee_distributions: string | null; } // Then use it: const row = result.data.result[0] as TransactionEventRow;This provides better IDE support and makes the expected structure explicit.
48-50: Include response details in error messages for better debugging.When the HTTP status is not 200, the error message doesn't include response body details that might help diagnose the issue.
if (result.status !== 200) { - throw new Error(`Failed to get transaction event: HTTP ${result.status}`); + throw new Error(`Failed to get transaction event: HTTP ${result.status}${result.data ? ': ' + JSON.stringify(result.data) : ''}`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/client/client.ts(2 hunks)src/contracts-api/transactionAction.ts(1 hunks)src/types/transaction.ts(1 hunks)tests/integration/transactionAction.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/integration/transactionAction.test.ts (1)
tests/integration/utils.ts (2)
testWithDefaultWallet(9-11)waitForTxSuccess(114-122)
src/client/client.ts (1)
src/contracts-api/transactionAction.ts (1)
TransactionAction(7-116)
src/contracts-api/transactionAction.ts (1)
src/types/transaction.ts (3)
GetTransactionEventInput(71-74)TransactionEvent(38-66)FeeDistribution(17-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (5)
src/client/client.ts (1)
205-214: LGTM! Consistent integration pattern.The
loadTransactionActionmethod follows the established pattern used by other loader methods in the class (loadAttestationAction,loadRoleManagementAction). The implementation correctly instantiatesTransactionActionwith the Kwil client and signer.tests/integration/transactionAction.test.ts (1)
8-241: Excellent test coverage for the new feature.The test suite comprehensively covers:
- Error handling (non-existent tx, empty txId)
- Transaction recording and retrieval for different methods
- Tx hash format flexibility (with/without 0x prefix)
- Transaction listing functionality
- Field validation and structure
The TODO comment (lines 223-240) appropriately documents the limitation around testing fee-paying wallets and references the alternative Go test coverage.
src/types/transaction.ts (1)
14-74: LGTM! Well-designed type definitions.The type definitions are well-structured with:
- Clear, comprehensive documentation explaining field relationships
- Proper handling of large numbers using string types for amounts
- Appropriate use of optional fields (
feeRecipient?,metadata?)- Detailed comments explaining when fields are undefined vs. populated
The documentation for
TransactionEvent(lines 27-37) is particularly helpful in clarifying the relationship betweenfeeAmount,feeRecipient, andfeeDistributions.src/contracts-api/transactionAction.ts (2)
16-31: LGTM! Clear documentation with practical example.The JSDoc comment provides excellent guidance with a concrete usage example. The description clearly explains the method's purpose, parameters, return value, and error conditions.
113-115: LGTM! Useful factory method.The
fromClientstatic factory method provides a convenient alternative constructor pattern, consistent with similar patterns in the codebase (e.g.,RoleManagement.fromClient).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/contracts-api/transactionAction.ts (1)
56-79: Validaterow.tx_idexists before using it in error messages.If the backend returns a malformed response where
tx_idis missing, subsequent error messages will display "(tx: undefined)" which is confusing. Validate thatrow.tx_idexists and is non-empty first.const row = result.data.result[0] as any; + + // Validate tx_id is present before using it in error messages + if (!row.tx_id || typeof row.tx_id !== 'string' || row.tx_id.trim() === '') { + throw new Error('Missing or invalid tx_id in response'); + } // Validate required fields
🧹 Nitpick comments (3)
src/contracts-api/transactionAction.ts (3)
1-1: Remove unused import.The
Typesimport from@trufnetwork/kwil-jsis not used in this file.-import { KwilSigner, NodeKwil, WebKwil, Types } from "@trufnetwork/kwil-js"; +import { KwilSigner, NodeKwil, WebKwil } from "@trufnetwork/kwil-js";
33-35: Consider adding format validation for transaction hash.While the backend may handle various formats, validating that
txIdis a valid hex string (with optional 0x prefix) would catch errors earlier and provide better user feedback.async getTransactionEvent(input: GetTransactionEventInput): Promise<TransactionEvent> { if (!input.txId || input.txId.trim() === "") { throw new Error("tx_id is required"); } + + // Validate txId format (hex string with optional 0x prefix) + const txIdTrimmed = input.txId.trim(); + const txIdWithoutPrefix = txIdTrimmed.startsWith('0x') ? txIdTrimmed.slice(2) : txIdTrimmed; + if (!/^[0-9a-fA-F]+$/.test(txIdWithoutPrefix)) { + throw new Error("tx_id must be a valid hex string"); + }
81-113: Well-structured fee distribution parsing.The parsing logic correctly handles the comma-separated format, uses
indexOfto find the first colon (handling addresses that may contain colons), and thoroughly validates each component including non-negative amounts.Optional enhancement: Consider validating that recipients are valid Ethereum addresses (0x-prefixed, 40 hex characters). Based on the type documentation, recipients should be Ethereum addresses.
// After line 94 const recipient = trimmedPart.substring(0, colonIndex).trim(); const amount = trimmedPart.substring(colonIndex + 1).trim(); // Add validation for Ethereum address format if (!/^0x[0-9a-fA-F]{40}$/i.test(recipient)) { throw new Error(`Invalid recipient address format: ${recipient} (tx: ${row.tx_id})`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/contracts-api/transactionAction.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/contracts-api/transactionAction.ts (1)
src/types/transaction.ts (3)
GetTransactionEventInput(71-74)TransactionEvent(38-66)FeeDistribution(17-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (5)
src/contracts-api/transactionAction.ts (5)
7-14: LGTM!The class structure and constructor are well-designed. Using protected fields allows for potential extension while maintaining encapsulation.
58-79: Excellent validation implementation.The required field validations comprehensively check type, presence, and validity. The non-negative check for
fee_amountand the string conversion ensure data consistency. This addresses the previous review comments effectively.
115-119: LGTM!Block height validation correctly ensures the value is a finite, non-negative integer.
121-131: LGTM!The return statement correctly maps all fields to the
TransactionEventinterface, using validated and normalized values. Optional fields are properly handled with undefined fallbacks.
133-142: LGTM!The static factory method provides a clean API for creating
TransactionActioninstances. The documentation clearly explains its purpose.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/integration/transactionAction.test.ts (1)
27-27: Consider reducing console.log statements.While console.log statements can be helpful for debugging integration tests, they create noise in test output. Consider removing them or using a structured logging approach that can be controlled via test configuration.
Alternatively, you could use vitest's built-in debugging capabilities or conditional logging based on environment variables:
const DEBUG = process.env.DEBUG_TESTS === 'true'; if (DEBUG) console.log(`✅ Successfully fetched deployStream transaction`);Also applies to: 40-40, 51-51, 60-60, 81-86, 100-100, 114-114, 135-140, 167-167, 218-219
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/contracts-api/transactionAction.ts(1 hunks)tests/integration/transactionAction.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/contracts-api/transactionAction.ts
🧰 Additional context used
🧬 Code graph analysis (1)
tests/integration/transactionAction.test.ts (1)
tests/integration/utils.ts (2)
testWithDefaultWallet(9-11)waitForTxSuccess(114-122)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (2)
tests/integration/transactionAction.test.ts (2)
1-13: LGTM!The test suite setup is well-structured. Using
describe.sequentialis appropriate for integration tests that interact with shared infrastructure, and the 6-minute timeout is reasonable for container-based testing.
15-241: Excellent test coverage for the TransactionAction feature!The test suite comprehensively covers:
- Error scenarios (non-existent transactions, empty inputs)
- Happy paths for different transaction types (deployStream, insertRecords)
- Edge cases (transaction hashes without 0x prefix)
- Alternative query methods (list_transaction_fees)
The TODO comment at lines 223-241 provides valuable context about known limitations and explains why fee-paying wallet tests are deferred. The tests properly validate transaction structure, field types, and integration with the broader SDK.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/integration/transactionAction.test.ts (1)
166-166: Remove type assertion to maintain type safety.Using
as anybypasses TypeScript's type checking. This was previously flagged in review comments.
🧹 Nitpick comments (1)
tests/integration/transactionAction.test.ts (1)
215-233: Document future test coverage needs.The TODO clearly explains the testing gap for fee-paying wallets and references the underlying technical constraints. The current test coverage for fee-exempt scenarios is appropriate.
Would you like me to open a new issue to track the fee-paying wallet test coverage once the kwil-js test utilities are available?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/integration/transactionAction.test.ts(1 hunks)tests/integration/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/integration/transactionAction.test.ts (1)
tests/integration/utils.ts (4)
setupTrufNetwork(177-177)testWithDefaultWallet(9-11)waitForTxSuccess(114-122)normalizeTransactionId(172-174)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (5)
tests/integration/utils.ts (1)
167-174: LGTM! Efficient implementation that addresses past feedback.The implementation successfully extracts the duplicated normalization logic and is more efficient than the suggested version—calling
.toLowerCase()only once instead of up to three times.tests/integration/transactionAction.test.ts (4)
15-42: LGTM! Comprehensive error handling coverage.Both tests properly validate error paths using appropriate assertions and provide clear debugging output.
44-136: LGTM! Thorough transaction validation tests.Both tests follow a solid pattern: execute transaction → wait for confirmation → query event → validate fields. The use of
normalizeTransactionIdcorrectly addresses the past review feedback, and the fee exemption behavior is well-documented.
138-161: LGTM! Prefix handling validated correctly.The test confirms the API accepts transaction hashes with or without the
0xprefix and always returns normalized output.
179-211: LGTM! Comprehensive validation of transaction listing.The test properly validates the response structure, ensures transactions are returned, and checks all expected fields. The logic for verifying transaction structure and content is thorough.
Signed-off-by: Michael Buntarman <michaelboentarman@gmail.com>
2 participants
Implement TransactionAction to query transaction fee events from the ledger. Enables applications to fetch transaction details including fee amounts, recipients, and distribution breakdowns.
resolves: https://github.com/trufnetwork/truf-network/issues/1331
Summary by CodeRabbit
New Features
Tests