From 1861d607dcf6ba2f3acf5500259d787c3259cf58 Mon Sep 17 00:00:00 2001 From: Luke Lee Date: Mon, 23 Dec 2024 12:20:20 -0800 Subject: [PATCH] feat: consolidate hbar transfer list when decoding cryptoTransfer function Signed-off-by: Luke Lee --- .../hts/transfer/ClassicTransfersDecoder.java | 23 ++++-- .../contract/impl/test/TestHelpers.java | 70 ++++++++++++++++++- .../transfer/ClassicTransfersDecoderTest.java | 70 +++++++++++++++++++ 3 files changed, 156 insertions(+), 7 deletions(-) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/systemcontracts/hts/transfer/ClassicTransfersDecoder.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/systemcontracts/hts/transfer/ClassicTransfersDecoder.java index a61ad87c899c..91a4fe761fe1 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/systemcontracts/hts/transfer/ClassicTransfersDecoder.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/systemcontracts/hts/transfer/ClassicTransfersDecoder.java @@ -103,7 +103,8 @@ public TransactionBody decodeCryptoTransfer( public TransactionBody decodeCryptoTransferV2( @NonNull final byte[] encoded, @NonNull final AddressIdConverter addressIdConverter) { final var call = ClassicTransfersTranslator.CRYPTO_TRANSFER_V2.decodeCall(encoded); - final var transferList = convertingMaybeApprovedAdjustments(((Tuple) call.get(0)).get(0), addressIdConverter); + final var transferList = consolidatedTransferList( + convertingMaybeApprovedAdjustments(((Tuple) call.get(0)).get(0), addressIdConverter)); final var cryptoTransfersBody = tokenTransfers(convertTokenTransfers( call.get(1), @@ -282,6 +283,14 @@ private CryptoTransferTransactionBody.Builder tokenTransfers(@NonNull TokenTrans return CryptoTransferTransactionBody.newBuilder().tokenTransfers(tokenTransferLists); } + private TransferList consolidatedTransferList(@NonNull final TransferList fromTransferList) { + final Map consolidatedTransfers = new LinkedHashMap<>(); + for (final var accountAmount : fromTransferList.accountAmounts()) { + consolidatedTransfers.merge(accountAmount.accountIDOrThrow(), accountAmount, this::mergeAdjusts); + } + return new TransferList(consolidatedTransfers.values().stream().toList()); + } + private TokenTransferList mergeTokenTransferLists( @NonNull final TokenTransferList from, @NonNull final TokenTransferList to) { return from.copyBuilder() @@ -308,10 +317,14 @@ private void consolidateInto( } private AccountAmount mergeAdjusts(@NonNull final AccountAmount from, @NonNull final AccountAmount to) { - return from.copyBuilder() - .amount(from.amount() + to.amount()) - .isApproval(from.isApproval() || to.isApproval()) - .build(); + try { + return from.copyBuilder() + .amount(Math.addExact(from.amount(), to.amount())) + .isApproval(from.isApproval() || to.isApproval()) + .build(); + } catch (ArithmeticException e) { + throw new IllegalArgumentException("Amount overflow when merging account amounts"); + } } private List mergeNftTransfers( diff --git a/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/TestHelpers.java b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/TestHelpers.java index 923f178e91fc..99d19b640487 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/TestHelpers.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/TestHelpers.java @@ -945,8 +945,9 @@ public static void givenConfigInFrame(@NonNull final MessageFrame frame, @NonNul given(frame.getMessageFrameStack()).willReturn(stack); } - /** Returns the test (mock) processor for all current EVM modules. - * + /** + * Returns the test (mock) processor for all current EVM modules. + *

* Needs to be updated when a new EVM module is created. */ public static Map processorsForAllCurrentEvmVersions( @@ -959,4 +960,69 @@ public static Map processorsForAllCurren HederaEvmVersion.VERSION_051, processor); } + + /** + * Helpful builder for creating a hbar transfer list + */ + public static class TransferListBuilder { + private Tuple transferList; + + public TransferListBuilder withAccountAmounts(final Tuple... accountAmounts) { + this.transferList = Tuple.singleton(accountAmounts); + return this; + } + + public Tuple build() { + return transferList; + } + } + + public static TransferListBuilder transferList() { + return new TransferListBuilder(); + } + + public static class TokenTransferListsBuilder { + private Tuple[] tokenTransferLists; + + public TokenTransferListsBuilder withTokenTransferList(final Tuple... tokenTransferLists) { + this.tokenTransferLists = tokenTransferLists; + return this; + } + + public Object build() { + return tokenTransferLists; + } + } + + public static TokenTransferListsBuilder tokenTransferLists() { + return new TokenTransferListsBuilder(); + } + + public static class TokenTransferListBuilder { + private Tuple tokenTransferList; + private com.esaulpaugh.headlong.abi.Address token; + + public TokenTransferListBuilder forToken(final com.esaulpaugh.headlong.abi.Address token) { + this.token = token; + return this; + } + + public TokenTransferListBuilder withAccountAmounts(final Tuple... accountAmounts) { + this.tokenTransferList = Tuple.of(token, accountAmounts, new Tuple[] {}); + return this; + } + + public TokenTransferListBuilder withNftTransfers(final Tuple... nftTransfers) { + this.tokenTransferList = Tuple.of(token, new Tuple[] {}, nftTransfers); + return this; + } + + public Tuple build() { + return tokenTransferList; + } + } + + public static TokenTransferListBuilder tokenTransferList() { + return new TokenTransferListBuilder(); + } } diff --git a/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/exec/systemcontracts/hts/transfer/ClassicTransfersDecoderTest.java b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/exec/systemcontracts/hts/transfer/ClassicTransfersDecoderTest.java index eb4ca6f57c0c..961d9b8fb371 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/exec/systemcontracts/hts/transfer/ClassicTransfersDecoderTest.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/exec/systemcontracts/hts/transfer/ClassicTransfersDecoderTest.java @@ -16,7 +16,13 @@ package com.hedera.node.app.service.contract.impl.test.exec.systemcontracts.hts.transfer; +import static com.hedera.node.app.service.contract.impl.test.TestHelpers.FUNGIBLE_TOKEN_HEADLONG_ADDRESS; +import static com.hedera.node.app.service.contract.impl.test.TestHelpers.tokenTransferList; +import static com.hedera.node.app.service.contract.impl.test.TestHelpers.tokenTransferLists; +import static com.hedera.node.app.service.contract.impl.test.TestHelpers.transferList; + import com.esaulpaugh.headlong.abi.Address; +import com.esaulpaugh.headlong.abi.Tuple; import com.hedera.hapi.node.base.AccountAmount; import com.hedera.hapi.node.base.AccountID; import com.hedera.hapi.node.transaction.TransactionBody; @@ -35,11 +41,13 @@ @ExtendWith(MockitoExtension.class) class ClassicTransfersDecoderTest { + public static final int ACCOUNT_ID_41 = 41; public static final int ACCOUNT_ID_42 = 42; private static final Address ACCT_ADDR_1 = addressFromNum(ACCOUNT_ID_41); private static final Address ACCT_ADDR_2 = addressFromNum(ACCOUNT_ID_42); private static final Address TOKEN_ADDR_10 = addressFromNum(10); + private static final Tuple[] EMPTY_TUPLE_ARRAY = new Tuple[] {}; @Mock private AddressIdConverter converter; @@ -93,12 +101,74 @@ void decodeHrcTransferFromHasCreditFirst() { Assertions.assertThat(secondEntry.amount()).isEqualTo(-totalToTransfer); } + @Test + void decodeCryptoTransferConsolidates() { + final var totalToTransfer = 25L; + BDDMockito.given(converter.convert(ACCT_ADDR_1)) + .willReturn(AccountID.newBuilder().accountNum(ACCOUNT_ID_41).build()); + BDDMockito.given(converter.convertCredit(ACCT_ADDR_1)) + .willReturn(AccountID.newBuilder().accountNum(ACCOUNT_ID_41).build()); + final var encodedInput = ClassicTransfersTranslator.CRYPTO_TRANSFER_V2.encodeCallWithArgs( + transferList() + .withAccountAmounts( + Tuple.of(ACCT_ADDR_1, -totalToTransfer, false), + Tuple.of(ACCT_ADDR_1, totalToTransfer, false)) + .build(), + EMPTY_TUPLE_ARRAY); + final var result = subject.decodeCryptoTransferV2(encodedInput.array(), converter); + Assertions.assertThat(unwrapTransferAmounts(result)).hasSize(1); + Assertions.assertThat(unwrapTransferAmounts(result).get(0).amount()).isEqualTo(0); + } + + @Test + void decodeCryptoTransferOverflow() { + BDDMockito.given(converter.convertCredit(ACCT_ADDR_1)) + .willReturn(AccountID.newBuilder().accountNum(ACCOUNT_ID_41).build()); + final var encodedInput = ClassicTransfersTranslator.CRYPTO_TRANSFER_V2.encodeCallWithArgs( + transferList() + .withAccountAmounts( + Tuple.of(ACCT_ADDR_1, Long.MAX_VALUE - 1, false), Tuple.of(ACCT_ADDR_1, 2L, false)) + .build(), + EMPTY_TUPLE_ARRAY); + Assertions.assertThatThrownBy(() -> subject.decodeCryptoTransferV2(encodedInput.array(), converter)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Amount overflow when merging account amounts"); + } + + @Test + void decodeCryptoTokenTransferOverflow() { + BDDMockito.given(converter.convertCredit(ACCT_ADDR_1)) + .willReturn(AccountID.newBuilder().accountNum(ACCOUNT_ID_41).build()); + final var encodedInput = ClassicTransfersTranslator.CRYPTO_TRANSFER_V2.encodeCallWithArgs( + transferList().withAccountAmounts().build(), + tokenTransferLists() + .withTokenTransferList( + tokenTransferList() + .forToken(FUNGIBLE_TOKEN_HEADLONG_ADDRESS) + .withAccountAmounts(Tuple.of(ACCT_ADDR_1, Long.MAX_VALUE - 1, false)) + .build(), + tokenTransferList() + .forToken(FUNGIBLE_TOKEN_HEADLONG_ADDRESS) + .withAccountAmounts(Tuple.of(ACCT_ADDR_1, 2L, false)) + .build()) + .build()); + Assertions.assertThatThrownBy(() -> subject.decodeCryptoTransferV2(encodedInput.array(), converter)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Amount overflow when merging account amounts"); + } + private static List unwrapTokenAmounts(final TransactionBody body) { final var tokenTransfers = body.cryptoTransfer().tokenTransfers(); // We shouldn't ever have more than one token transfer list return tokenTransfers.getFirst().transfers(); } + private static List unwrapTransferAmounts(final TransactionBody body) { + final var transfers = body.cryptoTransfer().transfers(); + // We shouldn't ever have more than one transfer list + return transfers.accountAmounts(); + } + private static Address addressFromNum(final long accountId) { return Address.wrap(Address.toChecksumAddress(BigInteger.valueOf(accountId))); }