Skip to content

Commit

Permalink
feat: consolidate hbar transfer list when decoding cryptoTransfer fun…
Browse files Browse the repository at this point in the history 
…ction

Signed-off-by: Luke Lee <[email protected]>
  • Loading branch information
@lukelee-sl
lukelee-sl committed Dec 24, 2024
1 parent aee0358 commit 1861d60
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 7 deletions.
23 changes: 18 additions & 5 deletions .../app/service/contract/impl/exec/systemcontracts/hts/transfer/ClassicTransfersDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -282,6 +283,14 @@ private CryptoTransferTransactionBody.Builder tokenTransfers(@NonNull TokenTrans
return CryptoTransferTransactionBody.newBuilder().tokenTransfers(tokenTransferLists);
}

private TransferList consolidatedTransferList(@NonNull final TransferList fromTransferList) {
final Map<AccountID, AccountAmount> 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()
Expand All @@ -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<NftTransfer> mergeNftTransfers(
Expand Down
70 changes: 68 additions & 2 deletions ...ervice-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/TestHelpers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
* <p>
* Needs to be updated when a new EVM module is created.
*/
public static Map<HederaEvmVersion, TransactionProcessor> processorsForAllCurrentEvmVersions(
Expand All @@ -959,4 +960,69 @@ public static Map<HederaEvmVersion, TransactionProcessor> 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();
}
}
70 changes: 70 additions & 0 deletions ...ice/contract/impl/test/exec/systemcontracts/hts/transfer/ClassicTransfersDecoderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<AccountAmount> 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<AccountAmount> 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)));
}
Expand Down

0 comments on commit 1861d60

Please sign in to comment.