feat(sdk-coin-icp): refactor account balance and fee retrieval to use IcpAgent class #6194
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the ICP account balance and fee retrieval logic to use a new IcpAgent class. Key changes include:
- Replacing getBalanceFromPrincipal stubs with IcpAgent methods in unit tests.
- Introducing a new IcpAgent class to encapsulate ICP ledger interactions.
- Updating the account balance and fee retrieval flow in the ICP coin implementation.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/sdk-coin-icp/test/unit/transactionBuilder/transactionRecover.ts | Updated unit tests to stub IcpAgent for balance and fee methods |
| modules/sdk-coin-icp/src/lib/utils.ts | Minor update to the JSDoc comment for feeData |
| modules/sdk-coin-icp/src/lib/iface.ts | Added new constants for fee and metadata retrieval |
| modules/sdk-coin-icp/src/lib/icpAgent.ts | New IcpAgent class for handling ledger interactions |
| modules/sdk-coin-icp/src/icp.ts | Refactored balance and fee retrieval logic to leverage IcpAgent |
| const actualBalance = balance.plus(feeData); // gas amount returned from gasData is negative so we add it | ||
| const balance = await this.getAccountBalance(publicKey); | ||
| const feeData = await this.getFeeData(); | ||
| const actualBalance = balance.minus(feeData); |
There was a problem hiding this comment.
The old comment referred to gas data being negative and added to the balance, but the new logic subtracts the fee. Consider updating or removing the outdated comment to match the current calculation.
Copilot uses AI. Check for mistakes.
| sinon.stub(IcpAgent.prototype, 'getBalance').resolves(BigNumber(1000000000)); | ||
| sinon.stub(IcpAgent.prototype, 'getFee').resolves(BigNumber(10000)); |
There was a problem hiding this comment.
[nitpick] Multiple tests repeat similar stubbing logic for IcpAgent. Consider centralizing these stubs in a beforeEach hook to reduce repetition and improve readability.
| sinon.stub(IcpAgent.prototype, 'getBalance').resolves(BigNumber(1000000000)); | |
| sinon.stub(IcpAgent.prototype, 'getFee').resolves(BigNumber(10000)); |
Copilot uses AI. Check for mistakes.
… IcpAgent class Ticket: Win-5302
Ticket: Win-5302