Skip to content

Commit

Permalink
chore: cherry-pick unified CryptoCreate throttle reclamation (#12339)
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Neeharika-Sompalli <[email protected]>
Co-authored-by: Neeharika-Sompalli <[email protected]>
  • Loading branch information
tinker-michaelj and Neeharika-Sompalli authored Mar 25, 2024
1 parent d515e92 commit abc8870
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.function.IntConsumer;
import javax.inject.Inject;
import javax.inject.Singleton;
import org.apache.commons.lang3.tuple.Pair;
Expand All @@ -75,7 +74,6 @@ public class TransferLogic {
private final TransactionContext txnCtx;
private final AliasManager aliasManager;
private final FeeDistribution feeDistribution;
private final IntConsumer cryptoCreateThrottleReclaimer;

@Inject
public TransferLogic(
Expand All @@ -89,8 +87,7 @@ public TransferLogic(
final RecordsHistorian recordsHistorian,
final TransactionContext txnCtx,
final AliasManager aliasManager,
final FeeDistribution feeDistribution,
IntConsumer cryptoCreateThrottleReclaimer) {
final FeeDistribution feeDistribution) {
this.tokenStore = tokenStore;
this.nftsLedger = nftsLedger;
this.accountsLedger = accountsLedger;
Expand All @@ -101,7 +98,6 @@ public TransferLogic(
this.txnCtx = txnCtx;
this.aliasManager = aliasManager;
this.feeDistribution = feeDistribution;
this.cryptoCreateThrottleReclaimer = cryptoCreateThrottleReclaimer;

scopedCheck = new MerkleAccountScopedCheck(validator, nftsLedger);
}
Expand All @@ -111,7 +107,6 @@ public void doZeroSum(final List<BalanceChange> changes) {
var validity = OK;
var autoCreationFee = 0L;
var updatedPayerBalance = Long.MIN_VALUE;
boolean failedAutoCreation = false;
boolean hasSuccessfulAutoCreation = false;
int numAutoCreationsSoFar = 0;
for (final var change : changes) {
Expand Down Expand Up @@ -144,7 +139,6 @@ public void doZeroSum(final List<BalanceChange> changes) {
} else {
validity = MAX_CHILD_RECORDS_EXCEEDED;
}
failedAutoCreation = validity != OK;
} else if (change.isForHbar()) {
validity = accountsLedger.validate(change.accountId(), scopedCheck.setBalanceChange(change));
if (change.affectsAccount(topLevelPayer)) {
Expand Down Expand Up @@ -186,9 +180,6 @@ public void doZeroSum(final List<BalanceChange> changes) {
if (autoCreationLogic != null && autoCreationLogic.reclaimPendingAliases()) {
accountsLedger.undoCreations();
}
if (failedAutoCreation && txnCtx.isSelfSubmitted()) {
cryptoCreateThrottleReclaimer.accept(txnCtx.numImplicitCreations());
}
throw new InvalidTransactionException(validity);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.hedera.node.app.service.mono.state.logic;

import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.FAIL_INVALID;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.SUCCESS;

import com.hedera.node.app.service.mono.context.TransactionContext;
import com.hedera.node.app.service.mono.context.properties.BootstrapProperties;
Expand All @@ -34,6 +35,7 @@
import edu.umd.cs.findbugs.annotations.NonNull;
import java.time.Instant;
import java.util.Objects;
import java.util.function.IntConsumer;
import javax.inject.Inject;
import javax.inject.Singleton;
import org.apache.logging.log4j.LogManager;
Expand All @@ -55,6 +57,7 @@ public class ServicesTxnManager {
private final MigrationRecordsManager migrationRecordsManager;
private final RecordStreaming recordStreaming;
private final BlockManager blockManager;
private final IntConsumer cryptoCreateThrottleReclaimer;
private final RewardCalculator rewardCalculator;
private final BootstrapProperties bootstrapProperties;
private final BlocklistAccountCreator blocklistAccountCreator;
Expand All @@ -71,6 +74,7 @@ public ServicesTxnManager(
final MigrationRecordsManager migrationRecordsManager,
final RecordStreaming recordStreaming,
final BlockManager blockManager,
final IntConsumer cryptoCreateThrottleReclaimer,
final RewardCalculator rewardCalculator,
final @NonNull BootstrapProperties bootstrapProperties,
final @NonNull BlocklistAccountCreator blocklistAccountCreator) {
Expand All @@ -84,6 +88,7 @@ public ServicesTxnManager(
this.migrationRecordsManager = migrationRecordsManager;
this.scopedTriggeredProcessing = scopedTriggeredProcessing;
this.blockManager = blockManager;
this.cryptoCreateThrottleReclaimer = cryptoCreateThrottleReclaimer;
this.rewardCalculator = rewardCalculator;
this.bootstrapProperties = Objects.requireNonNull(bootstrapProperties);
this.blocklistAccountCreator = Objects.requireNonNull(blocklistAccountCreator);
Expand Down Expand Up @@ -120,6 +125,9 @@ public void process(TxnAccessor accessor, Instant consensusTime, long submitting
} else {
scopedProcessing.run();
}
if (representsFailedAutoCreation(accessor) && txnCtx.isSelfSubmitted()) {
cryptoCreateThrottleReclaimer.accept(accessor.getNumImplicitCreations());
}
} catch (Exception processFailure) {
processFailed = true;
logContextualizedError(processFailure, "txn processing");
Expand All @@ -136,6 +144,10 @@ public void process(TxnAccessor accessor, Instant consensusTime, long submitting
}
}

private boolean representsFailedAutoCreation(TxnAccessor accessor) {
return txnCtx.status() != SUCCESS && accessor.getNumImplicitCreations() > 0;
}

private void attemptRecordStreaming() {
try {
recordStreaming.streamUserTxnRecords();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,14 @@ static TransactionalLedger<Pair<AccountID, TokenID>, TokenRelProperty, HederaTok
@Singleton
static IntConsumer provideCryptoCreateThrottleReclaimer(
@NonNull @HapiThrottle final FunctionalityThrottling hapiThrottling) {
return n -> hapiThrottling.leakCapacityForNOfUnscaled(n, CryptoCreate);
return n -> {
try {
hapiThrottling.leakCapacityForNOfUnscaled(n, CryptoCreate);
} catch (Exception ignore) {
// Ignore if the frontend bucket has already leaked all the capacity
// used for throttling the transaction on the frontend
}
};
}

@Provides
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,10 +240,10 @@ public TransferLogic newTransferLogic(
recordsHistorian,
txnCtx,
aliasManager,
feeDistribution,
feeDistribution
// No-op, we won't have used any frontend throttle capacity for an auto-creation
// attempted during an EVM transaction
n -> {});
);
}

public ApproveAllowanceLogic newApproveAllowanceLogic(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ public void rationalizeEthereumSpan(final TxnAccessor accessor) {
expandEthContext(accessor, workingState, spanMap, null);
accessor.setRationalizedSpanMap(spanMap);
}
accessor.countImplicitCreationsWith(aliasManager);
}

private boolean areChanged(final LinkedRefs linkedRefs) {
Expand Down Expand Up @@ -318,7 +319,7 @@ private void rationalizeImpliedTransfers(final TxnAccessor accessor) {
}
}

private void expandImpliedTransfers(final TxnAccessor accessor) {
public void expandImpliedTransfers(final TxnAccessor accessor) {
final var op = accessor.getTxn().getCryptoTransfer();
final var impliedTransfers = impliedTransfersMarshal.unmarshalFromGrpc(op, accessor.getPayer());
reCalculateXferMeta(accessor, impliedTransfers);
Expand All @@ -328,12 +329,13 @@ private void expandImpliedTransfers(final TxnAccessor accessor) {
}

public static void reCalculateXferMeta(final TxnAccessor accessor, final ImpliedTransfers impliedTransfers) {
final var maybeAssessedCustomFees = impliedTransfers.getAssessedCustomFeeWrappers();
final var xferMeta = accessor.availXferUsageMeta();

var customFeeTokenTransfers = 0;
var customFeeHbarTransfers = 0;
final Set<EntityId> involvedTokens = new HashSet<>();
for (final var assessedFeeWrapper : impliedTransfers.getAssessedCustomFeeWrappers()) {
for (final var assessedFeeWrapper : maybeAssessedCustomFees) {
if (assessedFeeWrapper.isForHbar()) {
customFeeHbarTransfers++;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;

import com.hedera.node.app.service.evm.exceptions.InvalidTransactionException;
Expand Down Expand Up @@ -200,8 +199,7 @@ void setUp() {
historian,
txnCtx,
aliasManager,
feeDistribution,
cryptoCreateThrottleReclaimer);
feeDistribution);

subject = new HederaLedger(
tokenStore,
Expand Down Expand Up @@ -308,8 +306,7 @@ void rejectsMissingToken() {
historian,
txnCtx,
aliasManager,
feeDistribution,
cryptoCreateThrottleReclaimer);
feeDistribution);
subject = new HederaLedger(
tokenStore,
ids,
Expand Down Expand Up @@ -486,15 +483,12 @@ void invalidTransfersWithAutoCreationDrainsCapacityIfSelfSubmitted() {
backingAccounts.put(payer, payerAccount);

given(txnCtx.activePayer()).willReturn(payer);
given(txnCtx.numImplicitCreations()).willReturn(2);
given(txnCtx.isSelfSubmitted()).willReturn(true);
given(autoCreationLogic.create(any(), eq(accountsLedger), any())).willReturn(Pair.of(INVALID_ACCOUNT_ID, 100L));
given(aliasManager.lookupIdBy(aliasA.toByteString())).willReturn(EntityNum.MISSING_NUM);
given(historian.canTrackPrecedingChildRecords(anyInt())).willReturn(true);

subject.begin();
assertFailsWith(() -> subject.doZeroSum(changes), INVALID_ACCOUNT_ID);
verify(cryptoCreateThrottleReclaimer).accept(2);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,7 @@ void setUp() {
recordsHistorian,
txnCtx,
aliasManager,
feeDistribution,
cryptoCreateThrottleReclaimer);
feeDistribution);
}

@Test
Expand All @@ -197,8 +196,7 @@ void throwsIseOnNonEmptyAliasWithNullAutoCreationLogic() {
recordsHistorian,
txnCtx,
aliasManager,
feeDistribution,
cryptoCreateThrottleReclaimer);
feeDistribution);

final var triggerList = List.of(inappropriateTrigger);
assertThrows(IllegalStateException.class, () -> subject.doZeroSum(triggerList));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import com.hederahashgraph.api.proto.java.ResponseCodeEnum;
import com.hederahashgraph.api.proto.java.Transaction;
import java.time.Instant;
import java.util.function.IntConsumer;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -68,6 +69,9 @@ class ServicesTxnManagerTest {
@Mock
private SignedTxnAccessor accessor;

@Mock
private IntConsumer cryptoCreateThrottleReclaimer;

@Mock
private HederaLedger ledger;

Expand Down Expand Up @@ -120,6 +124,7 @@ void setup() {
migrationRecordsManager,
recordStreaming,
blockManager,
cryptoCreateThrottleReclaimer,
rewardCalculator,
bootstrapProperties,
blocklistAccountCreator);
Expand Down

0 comments on commit abc8870

Please sign in to comment.