Skip to content

test(hbar): add unit tests for CoinTransferBuilder self-transfer#8742

Merged
Doddanna17 merged 1 commit into
masterfrom
SI-539-hbar-self-transfer-unit-test
May 13, 2026
Merged

test(hbar): add unit tests for CoinTransferBuilder self-transfer#8742
Doddanna17 merged 1 commit into
masterfrom
SI-539-hbar-self-transfer-unit-test

Conversation

@Doddanna17
Copy link
Copy Markdown
Contributor

Summary

  • Adds unit tests for CoinTransferBuilder self-transfer (sender == recipient), covering the stakeClaimRewards 1-tinybar claim-rewards mechanism
  • Verifies two separate accountAmounts entries are preserved in the protobuf (bypassing Hedera SDK same-account netting)
  • Covers serialisation round-trip and signing

Test plan

  • yarn mocha --grep "HBAR CoinTransferBuilder" passes all 4 test cases in coinTransferBuilder.ts
  • CI unit test suite passes for sdk-coin-hbar

Ticket: SI-539

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 12, 2026

SI-539

HBAR claim rewards uses a 1-tinybar self-transfer (sender == recipient).
getTransferData() filtered out the positive-amount entry when its
accountID matched the sender, leaving recipients[] empty. toJson() then
crashed reading recipients[0].address.

Fix: after the forEach loop, if transferData is still empty and transfers
exist, include the positive-amount entry as the sole recipient. Normal
transfers are unaffected.

Also adds unit tests covering: build with source == recipient, two
separate accountAmounts protobuf entries, serialisation round-trip,
and signing.

Ticket: SI-539
@Doddanna17 Doddanna17 force-pushed the SI-539-hbar-self-transfer-unit-test branch from e17b3a6 to fcf0884 Compare May 12, 2026 10:11
@Doddanna17 Doddanna17 marked this pull request as ready for review May 13, 2026 05:55
@Doddanna17 Doddanna17 requested a review from a team as a code owner May 13, 2026 05:55
@Doddanna17
Copy link
Copy Markdown
Contributor Author

HBAR Claim Rewards — Self-Transfer Implementation Notes

Background

Hedera has no dedicated "claim rewards" transaction type at the protocol level. Staking rewards are harvested automatically whenever a CryptoTransfer touches the account. The standard approach is a 1-tinybar self-transfer (sender == recipient == accountId), which atomically flushes pending_reward into the account balance.

Data flow

staking-service
  HbarClaimRewardsRequestStrategy → amount = 1 tinybar, self-transfer intent

bitgo-microservices (hbar.ts — existing, no changes needed)
  TxType.ClaimRewards → TxType.Transfer
  recipients = [{ address: walletBaseAddress, amount: '1' }]

HederaFactory → getTransferBuilder() → CoinTransferBuilder
  buildTransferData(): creates proto accountAmounts:
    [{ accountId: X, amount: -1 }, { accountId: X, amount: +1 }]
    (same accountId for both entries — intentional self-transfer)

Transaction sent to Hedera → reward flush triggers automatically

Transaction comes back for parsing/signing:
  TransactionBuilderFactory.from() → TransactionType.Send → getTransferBuilder()
  transaction.ts getTransferData()
    Before this fix: filters out the positive entry (accountID === sender)
                     → recipients[] empty → crash at recipients[0].address
    After this fix:  detects empty recipients + existing transfers → self-transfer fallback
                     → returns recipients[{ address: accountId, amount: '1' }]
  toJson() → recipients[0].address works correctly

Why no TransactionType.StakingClaim is needed

Other coins (ADA, TRX, SUI) use TransactionType.StakingClaim with a dedicated getStakingClaimRewardsBuilder(). HBAR doesn't because the claim is structurally identical to a regular transfer — the existing CoinTransferBuilder handles it. The only thing that was missing was the getTransferData() fix for the self-transfer case.

Changes in this PR

File Change
transaction.ts Self-transfer fallback in getTransferData() — when all positive-amount entries belong to the sender (sender == recipient), include the positive entry as the recipient instead of returning empty
coinTransferBuilder.ts 4 unit tests covering: build self-transfer, proto produces two separate accountAmounts entries, serialization round-trip, signing

No statics changes required. No new builder factory cases required.

@raksha-r7
Copy link
Copy Markdown
Contributor

@claude please review

Copy link
Copy Markdown
Contributor

@raksha-r7 raksha-r7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Consider adding a test or validation for the amount: '0' self-transfer edge case, since getTransferData() would return empty recipients in that scenario (the isPositive() check excludes zero).
otherwise lgtm

@Doddanna17 Doddanna17 merged commit c1af298 into master May 13, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants