Skip to content

Commit

Permalink
fix: immediately finalize transfer lists for scheduled crypto transfer (
Browse files Browse the repository at this point in the history
#14799)

Signed-off-by: Michael Tinker <[email protected]>
  • Loading branch information
tinker-michaelj authored Aug 12, 2024
1 parent 8904f7b commit 915337a
Show file tree
Hide file tree
Showing 5 changed files with 208 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,10 @@ private RecordDispatch newChildDispatch(
new WritableStoreFactory(stack, TokenService.NAME, config, storeMetricsService),
recordBuilder,
consensusNow,
config),
config,
txnInfo.functionality(),
category,
recordListBuilder),
recordListBuilder,
platformState,
preHandleResult);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,18 @@

package com.hedera.node.app.workflows.handle.record;

import static com.hedera.hapi.node.base.HederaFunctionality.CRYPTO_TRANSFER;
import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.SCHEDULED;
import static java.util.Objects.requireNonNull;

import com.hedera.hapi.node.base.HederaFunctionality;
import com.hedera.node.app.service.token.records.FinalizeContext;
import com.hedera.node.app.spi.workflows.HandleContext;
import com.hedera.node.app.store.ReadableStoreFactory;
import com.hedera.node.app.store.WritableStoreFactory;
import com.swirlds.config.api.Configuration;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.time.Instant;
import java.util.Objects;
import java.util.function.Consumer;

/**
Expand All @@ -33,16 +38,25 @@
public class TriggeredFinalizeContext extends ChildFinalizeContextImpl implements FinalizeContext {
private final Instant consensusNow;
private final Configuration configuration;
private final HederaFunctionality functionality;
private final HandleContext.TransactionCategory category;
private final RecordListBuilder recordListBuilder;

public TriggeredFinalizeContext(
@NonNull final ReadableStoreFactory readableStoreFactory,
@NonNull final WritableStoreFactory writableStoreFactory,
@NonNull final SingleTransactionRecordBuilderImpl recordBuilder,
@NonNull final Instant consensusNow,
@NonNull final Configuration configuration) {
@NonNull final Configuration configuration,
@NonNull final HederaFunctionality functionality,
@NonNull final HandleContext.TransactionCategory category,
@NonNull final RecordListBuilder recordListBuilder) {
super(configuration, readableStoreFactory, writableStoreFactory, recordBuilder);
this.consensusNow = Objects.requireNonNull(consensusNow);
this.configuration = Objects.requireNonNull(configuration);
this.consensusNow = requireNonNull(consensusNow);
this.configuration = requireNonNull(configuration);
this.functionality = requireNonNull(functionality);
this.category = requireNonNull(category);
this.recordListBuilder = requireNonNull(recordListBuilder);
}

@NonNull
Expand All @@ -59,14 +73,30 @@ public Configuration configuration() {

@Override
public boolean hasChildOrPrecedingRecords() {
// Since this is only used for a scheduled dispatch, we should not deduct any changes from this transaction
// So always return false.
return false;
// There is a single case in 0.52 where a child dispatch can itself have a logical child with non-zero
// balance adjustments---a scheduled crypto transfer that triggers an auto-account creation
if (category == SCHEDULED && functionality == CRYPTO_TRANSFER) {
final var precedingBuilders = recordListBuilder.precedingRecordBuilders();
return precedingBuilders.stream()
.anyMatch(
builder -> !builder.transferList().accountAmounts().isEmpty());
} else {
return false;
}
}

@Override
public <T> void forEachChildRecord(@NonNull Class<T> recordBuilderClass, @NonNull Consumer<T> consumer) {
// No-op, as contract operations cannot be scheduled at this time
public <T> void forEachChildRecord(
@NonNull final Class<T> recordBuilderClass, @NonNull final Consumer<T> consumer) {
requireNonNull(recordBuilderClass);
requireNonNull(consumer);
if (category == SCHEDULED && functionality == CRYPTO_TRANSFER) {
final var precedingBuilders = recordListBuilder.precedingRecordBuilders();
precedingBuilders.stream()
.filter(builder -> !builder.transferList().accountAmounts().isEmpty())
.map(recordBuilderClass::cast)
.forEach(consumer);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
* Copyright (C) 2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.hedera.node.app.workflows.handle.record;

import static com.hedera.hapi.node.base.HederaFunctionality.CRYPTO_TRANSFER;
import static com.hedera.hapi.node.base.HederaFunctionality.TOKEN_MINT;
import static com.hedera.node.app.fixtures.AppTestBase.DEFAULT_CONFIG;
import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.CHILD;
import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.SCHEDULED;
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;

import com.hedera.hapi.node.base.AccountAmount;
import com.hedera.hapi.node.base.HederaFunctionality;
import com.hedera.hapi.node.base.TransferList;
import com.hedera.node.app.service.token.records.ChildRecordBuilder;
import com.hedera.node.app.spi.workflows.HandleContext;
import com.hedera.node.app.store.ReadableStoreFactory;
import com.hedera.node.app.store.WritableStoreFactory;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.time.Instant;
import java.util.List;
import java.util.function.Consumer;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
class TriggeredFinalizeContextTest {
private static final Instant NOW = Instant.ofEpochSecond(1_234_567L);

@Mock
private RecordListBuilder recordListBuilder;

@Mock
private ReadableStoreFactory readableStoreFactory;

@Mock
private WritableStoreFactory writableStoreFactory;

@Mock
private SingleTransactionRecordBuilderImpl recordBuilder;

@Mock
private SingleTransactionRecordBuilderImpl childNoAdjustments;

@Mock
private SingleTransactionRecordBuilderImpl childWithAdjustments;

@Mock
private Consumer<ChildRecordBuilder> consumer;

private TriggeredFinalizeContext subject;

@Test
void nonScheduledChildrenHaveNoSelfChildren() {
subject = subjectWith(CHILD, CRYPTO_TRANSFER);
subject.forEachChildRecord(ChildRecordBuilder.class, consumer);

assertThat(subject.hasChildOrPrecedingRecords()).isFalse();
verifyNoInteractions(recordListBuilder);
verifyNoInteractions(consumer);
}

@Test
void scheduledMintChildrenHaveNoSelfChildren() {
subject = subjectWith(SCHEDULED, TOKEN_MINT);
subject.forEachChildRecord(ChildRecordBuilder.class, consumer);

assertThat(subject.hasChildOrPrecedingRecords()).isFalse();
verifyNoInteractions(recordListBuilder);
verifyNoInteractions(consumer);
}

@Test
void scheduledCryptoTransferHasChildrenIfBalancesChange() {
given(childNoAdjustments.transferList()).willReturn(TransferList.DEFAULT);
given(childWithAdjustments.transferList())
.willReturn(new TransferList(List.of(AccountAmount.DEFAULT, AccountAmount.DEFAULT)));
given(recordListBuilder.precedingRecordBuilders())
.willReturn(List.of(childNoAdjustments, childWithAdjustments));
subject = subjectWith(SCHEDULED, CRYPTO_TRANSFER);
subject.forEachChildRecord(ChildRecordBuilder.class, consumer);

assertThat(subject.hasChildOrPrecedingRecords()).isTrue();
verify(consumer).accept(childWithAdjustments);
verify(consumer, never()).accept(childNoAdjustments);
}

private TriggeredFinalizeContext subjectWith(
@NonNull final HandleContext.TransactionCategory category,
@NonNull final HederaFunctionality functionality) {
return new TriggeredFinalizeContext(
readableStoreFactory,
writableStoreFactory,
recordBuilder,
NOW,
DEFAULT_CONFIG,
functionality,
category,
recordListBuilder);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
import com.hedera.services.bdd.spec.HapiSpec;
import com.hedera.services.bdd.spec.fees.TinyBarTransfers;
import com.hedera.services.bdd.spec.transactions.TxnUtils;
import com.hederahashgraph.api.proto.java.AccountAmount;
import com.hederahashgraph.api.proto.java.AccountID;
import com.hederahashgraph.api.proto.java.TransferList;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.util.AbstractMap;
import java.util.Arrays;
import java.util.HashSet;
Expand Down Expand Up @@ -66,6 +68,15 @@ public static TransferListAsserts including(Function<HapiSpec, TransferList>...
return new ExplicitTransferAsserts(Arrays.asList(providers));
}

public static Function<HapiSpec, TransferList> adjustment(@NonNull final String account, final long amount) {
return spec -> TransferList.newBuilder()
.addAccountAmounts(AccountAmount.newBuilder()
.setAccountID(asId(account, spec))
.setAmount(amount)
.build())
.build();
}

public static TransferListAsserts includingDeduction(LongSupplier from, long amount) {
return new DeductionAsserts(from, amount);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,12 @@
import static com.hedera.services.bdd.junit.TestTags.NOT_REPEATABLE;
import static com.hedera.services.bdd.spec.HapiSpec.customHapiSpec;
import static com.hedera.services.bdd.spec.HapiSpec.defaultHapiSpec;
import static com.hedera.services.bdd.spec.HapiSpec.hapiTest;
import static com.hedera.services.bdd.spec.HapiSpec.propertyPreservingHapiSpec;
import static com.hedera.services.bdd.spec.assertions.AccountInfoAsserts.accountWith;
import static com.hedera.services.bdd.spec.assertions.TransactionRecordAsserts.recordWith;
import static com.hedera.services.bdd.spec.assertions.TransferListAsserts.adjustment;
import static com.hedera.services.bdd.spec.assertions.TransferListAsserts.including;
import static com.hedera.services.bdd.spec.keys.ControlForKey.forKey;
import static com.hedera.services.bdd.spec.keys.KeyShape.SIMPLE;
import static com.hedera.services.bdd.spec.keys.KeyShape.listOf;
Expand Down Expand Up @@ -108,6 +112,34 @@
import org.junit.jupiter.api.Tag;

public class ScheduleCreateSpecs {
@HapiTest
final Stream<DynamicTest> scheduledAutoCreationWithCustomPayerHasExpectedRecords() {
final var customPayer = "customPayer";
return hapiTest(
newKeyNamed(ALIAS),
cryptoCreate(PAYER).balance(10 * ONE_HUNDRED_HBARS),
cryptoCreate(customPayer).payingWith(PAYER).balance(20 * ONE_HBAR),
scheduleCreate(
"autoCreationWithCustomPayer",
cryptoTransfer(tinyBarsFromToWithAlias(PAYER, ALIAS, ONE_HBAR)))
.designatingPayer(customPayer)
.payingWith(PAYER)
.via("autoCreationWithCustomPayer")
.fee(ONE_HBAR),
scheduleSign("autoCreationWithCustomPayer")
.payingWith(customPayer)
.via("signTxn"),
getTxnRecord("signTxn")
.andAllChildRecords()
.hasPriority(recordWith()
.transfers(including(
adjustment("0.0.3", 25768),
adjustment("0.0.98", 391354),
adjustment("0.0.800", 48919),
adjustment("0.0.801", 48919),
adjustment(customPayer, -514960)))));
}

@HapiTest
final Stream<DynamicTest> aliasNotAllowedAsPayer() {
return defaultHapiSpec("BodyAndPayerCreation")
Expand Down

0 comments on commit 915337a

Please sign in to comment.