test: add additional tests for HIP-904 #17773

bubo · 2025-02-07T15:54:04Z

Description:
Tests from the Airdrop to Contract test plan - 8, 9, 11, 12

Related issue(s):

Fixes #

#16269
#16270
#16272
#16273

Notes for reviewer:

Checklist

Documented (Code comments, README, etc.)
Tested (unit, integration, etc.)

codecov · 2025-02-07T16:06:24Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.01%. Comparing base (a2f4da1) to head (fb5fc76).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main   #17773   +/-   ##
=========================================
  Coverage     69.01%   69.01%           
  Complexity    23036    23036           
=========================================
  Files          2654     2654           
  Lines         99652    99652           
  Branches      10284    10284           
=========================================
  Hits          68777    68777           
  Misses        26976    26976           
  Partials       3899     3899

codacy-production · 2025-02-07T16:11:16Z

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ +0.00% (target: -1.00%)	✅ ∅

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (`a2f4da1`)	99435	72515	72.93%
Head commit (`fb5fc76`)	99435 (+0)	72515 (+0)	72.93% (+0.00%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#17773)	0	0	∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings Change summary preferences

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}

Signed-off-by: Lyuben Ivanov <[email protected]>

stoyanov-st

Good job so far.
Left a few comments for more verifications and one of the tests needs a bit more work to deploy contract with create2

stoyanov-st · 2025-02-11T11:47:14Z

...ava/com/hedera/services/bdd/suites/contract/precompile/airdrops/AirdropFromContractTest.java

+        final var hollowAccountKey = "hollowAccountKey";
+        final AtomicReference<ByteString> hollowAccountAlias = new AtomicReference<>();
+
+        return hapiTest(


This test only airdrops to a hollow account. You can check Create2OperationSuite to get a reference on how to do the create2 part with a hapi test.

Actually the description and the method name here were wrong, this test does not need create2 contract.

stoyanov-st · 2025-02-11T11:48:53Z

...ava/com/hedera/services/bdd/suites/contract/precompile/airdrops/AirdropFromContractTest.java

+                            .call("tokenAirdrop", token, sender, receiver, 5L)
+                            .sending(85_000_000L)
+                            .gas(1_500_000L),
+                    receiver.getBalance().andAssert(balance -> balance.hasTokenBalance(token.name(), 0L)),


You can also verify the PendingAirdrop from the record

The check is added.

also add the including*PendingAirdrop

stoyanov-st · 2025-02-11T11:49:48Z

...ava/com/hedera/services/bdd/suites/contract/precompile/airdrops/AirdropFromContractTest.java

+                            .via("AirdropTxn"),
+                    receiver.getBalance().andAssert(balance -> balance.hasTokenBalance(token1.name(), 0L)),
+                    receiver.getBalance().andAssert(balance -> balance.hasTokenBalance(token2.name(), 0L)),
+                    getTxnRecord("AirdropTxn").hasChildRecords(recordWith().pendingAirdropsCount(2)));


Here also verify the PendingAirdrops apart from the counter

Signed-off-by: Lyuben Ivanov <[email protected]>

stoyanov-st

Needs a few more adjustments.
Try to be more explicit in the assertions and add comments if needed, this helps a lot with the reviews and understanding of what is being tested.

stoyanov-st · 2025-02-13T08:36:50Z

...ava/com/hedera/services/bdd/suites/contract/precompile/airdrops/AirdropFromContractTest.java

+                    getTxnRecord("AirdropTxn").hasChildRecords(recordWith().pendingAirdropsCount(2)),
+                    getTxnRecord("AirdropTxn")
+                            .hasChildRecords(recordWith()
+                                    .pendingAirdrops(includingFungiblePendingAirdrop(


No need to get the record again, you can just chain the pending checks

stoyanov-st · 2025-02-13T08:37:55Z

...ava/com/hedera/services/bdd/suites/contract/precompile/airdrops/AirdropFromContractTest.java

+                            .call("tokenAirdrop", token, sender, receiver, 5L)
+                            .sending(85_000_000L)
+                            .gas(1_500_000L),
+                    receiver.getBalance().andAssert(balance -> balance.hasTokenBalance(token.name(), 0L)),


also add the including*PendingAirdrop

stoyanov-st · 2025-02-13T08:40:02Z

...ava/com/hedera/services/bdd/suites/contract/precompile/airdrops/AirdropFromContractTest.java

+                            .sending(85_000_000L)
+                            .gas(1_500_000L)
+                            .via("AirdropTxn"),
+                    getTxnRecord("AirdropTxn").hasChildRecords(recordWith().pendingAirdropsCount(1)),


stoyanov-st · 2025-02-13T08:40:53Z

...ava/com/hedera/services/bdd/suites/contract/precompile/airdrops/AirdropFromContractTest.java

+                            .sending(85_000_000L)
+                            .gas(1_500_000L)
+                            .via("AirdropTxn"),
+                    getTxnRecord("AirdropTxn").hasChildRecords(recordWith().pendingAirdropsCount(1)),


stoyanov-st · 2025-02-13T08:42:54Z

...ava/com/hedera/services/bdd/suites/contract/precompile/airdrops/AirdropFromContractTest.java

+                            .sending(85_000_000L)
+                            .gas(1_500_000L)
+                            .via("AirdropTxn"),
+                    getTxnRecord("AirdropTxn").hasChildRecords(recordWith().pendingAirdropsCount(0)),


Use different transaction ids as this might result in race conditions and leaky tests

stoyanov-st · 2025-02-13T08:48:14Z

...ava/com/hedera/services/bdd/suites/contract/precompile/airdrops/AirdropFromContractTest.java

+                    token.treasury().transferUnitsTo(sender2, 10L, token));
+            allRunFor(
+                    spec,
+                    airdropContract


Here it should be only one airdrop with multiple senders and receivers.
You can check the multiAirdrops() tests in AirdropToContractSystemContractTest

stoyanov-st · 2025-02-13T08:49:48Z

...ava/com/hedera/services/bdd/suites/contract/precompile/airdrops/AirdropFromContractTest.java

+                            .call("tokenAirdrop", token, sender, receiver, 1L)
+                            .sending(85_000_000L)
+                            .gas(1_500_000L)
+                            .via("AirdropTxn"),


Also the transaction ids should be different

bubo added this to the v0.60 milestone Feb 7, 2025

bubo self-assigned this Feb 7, 2025

bubo requested review from a team as code owners February 7, 2025 15:54

bubo requested a review from thomas-swirlds-labs February 7, 2025 15:54

bubo changed the title ~~tests: add additional tests for 904~~ tests: add additional tests for HIP-904 Feb 7, 2025

test: add additional tests for HIP-904

425403c

Signed-off-by: Lyuben Ivanov <[email protected]>

bubo force-pushed the hip-904-airdrop-from-contract branch from bb18d69 to 425403c Compare February 10, 2025 11:55

bubo changed the title ~~tests: add additional tests for HIP-904~~ test: add additional tests for HIP-904 Feb 10, 2025

bubo closed this Feb 10, 2025

bubo reopened this Feb 10, 2025

stoyanov-st reviewed Feb 11, 2025

View reviewed changes

bubo added 3 commits February 11, 2025 17:38

added 3 more tests for HIP-904, N 13,14,15 from #15596

136ee65

Signed-off-by: Lyuben Ivanov <[email protected]>

fix: address PR comments

274b567

Signed-off-by: Lyuben Ivanov <[email protected]>

Merge branch 'main' into hip-904-airdrop-from-contract

fb5fc76

stoyanov-st reviewed Feb 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add additional tests for HIP-904 #17773

test: add additional tests for HIP-904 #17773

bubo commented Feb 7, 2025 •

edited

Loading

codecov bot commented Feb 7, 2025 •

edited

Loading

codacy-production bot commented Feb 7, 2025 •

edited

Loading

stoyanov-st left a comment

stoyanov-st Feb 11, 2025

bubo Feb 12, 2025

stoyanov-st Feb 11, 2025

bubo Feb 12, 2025

stoyanov-st Feb 13, 2025

stoyanov-st Feb 11, 2025

bubo Feb 12, 2025

stoyanov-st left a comment

stoyanov-st Feb 13, 2025

stoyanov-st Feb 13, 2025

stoyanov-st Feb 13, 2025

stoyanov-st Feb 13, 2025

stoyanov-st Feb 13, 2025

stoyanov-st Feb 13, 2025

stoyanov-st Feb 13, 2025

test: add additional tests for HIP-904 #17773

Are you sure you want to change the base?

test: add additional tests for HIP-904 #17773

Conversation

bubo commented Feb 7, 2025 • edited Loading

codecov bot commented Feb 7, 2025 • edited Loading

Codecov Report

codacy-production bot commented Feb 7, 2025 • edited Loading

Coverage summary from Codacy

stoyanov-st left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stoyanov-st left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bubo commented Feb 7, 2025 •

edited

Loading

codecov bot commented Feb 7, 2025 •

edited

Loading

codacy-production bot commented Feb 7, 2025 •

edited

Loading