Skip to content

Commit

Permalink
Enable HollowAccountFinalizationSuite (#9535)
Browse files Browse the repository at this point in the history 
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <122310714+iwsimon@users.noreply.github.com>
Signed-off-by: Neeharika-Sompalli <neeharika.sompalli@swirldslabs.com>
Signed-off-by: Neeharika Sompalli <52669918+Neeharika-Sompalli@users.noreply.github.com>
Signed-off-by: Michael Heinrichs <netopyr@users.noreply.github.com>
Co-authored-by: Iris Simon <iris.simon@swirldslabs.com>
Co-authored-by: Iris Simon <122310714+iwsimon@users.noreply.github.com>
Co-authored-by: Michael Heinrichs <netopyr@users.noreply.github.com>
  • Loading branch information
@Neeharika-Sompalli @iwsimon @netopyr
4 people committed Nov 6, 2023
1 parent baa5178 commit 20bdd4a
Show file tree
Hide file tree
Showing 34 changed files with 398 additions and 150 deletions.
33 changes: 32 additions & 1 deletion ...ra-node/hedera-app-spi/src/main/java/com/hedera/node/app/spi/workflows/HandleContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,37 @@ <T> T dispatchReversiblePrecedingTransaction(
@NonNull Predicate<Key> verifier,
AccountID syntheticPayer);

/**
* Dispatches preceding transaction that can be removed.
*
* <p>A removable preceding transaction depends on the current transaction. That means if the user transaction
* fails, a removable preceding transaction is automatically removed and not exported. The state changes introduced by a
* removable preceding transaction are automatically committed together with the parent transaction.
*
* <p>This method can only be called by a {@link TransactionCategory#USER}-transaction and only as long as no state
* changes have been introduced by the user transaction (either by storing state or by calling a child
* transaction).
*
* <p>The provided {@link Predicate} callback will be called to verify simple keys when the child transaction calls
* any of the {@code verificationFor} methods.
*
* @param txBody the {@link TransactionBody} of the transaction to dispatch
* @param recordBuilderClass the record builder class of the transaction
* @param verifier a {@link Predicate} that will be used to validate primitive keys
* @param syntheticPayer the payer of the transaction
* @return the record builder of the transaction
* @throws NullPointerException if {@code txBody} is {@code null}
* @throws IllegalArgumentException if the transaction is not a {@link TransactionCategory#USER}-transaction or if
* the record builder type is unknown to the app
* @throws IllegalStateException if the current transaction has already introduced state changes
*/
@NonNull
<T> T dispatchRemovablePrecedingTransaction(
@NonNull TransactionBody txBody,
@NonNull Class<T> recordBuilderClass,
@NonNull Predicate<Key> verifier,
AccountID syntheticPayer);

/**
* Dispatches a reversible preceding transaction that already has an ID.
*
Expand Down Expand Up @@ -603,7 +634,7 @@ default <T> T dispatchRemovableChildTransaction(
SavepointStack savepointStack();

/**
* Revert all child records in {@link RecordListBuilder}.
* Revert all child records in RecordListBuilder.
*/
void revertChildRecords();

Expand Down
5 changes: 4 additions & 1 deletion ...-node/hedera-app/src/main/java/com/hedera/node/app/state/recordcache/RecordCacheImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,10 @@ private void addToInMemoryCache(
// And all transactions, regardless of the type, are added to the payer-reverse-index, so that queries of
// the payer account ID will return all transactions they paid for.
final var txId = transactionRecord.transactionIDOrThrow();
final var isChildTx = transactionRecord.hasParentConsensusTimestamp();
// For the preceding child records parentConsensusTimestamp is not set, but the nonce will be greater than 1
// For the following child records parentConsensusTimestamp is also set. So to differentiate child records
// from user records, we check if the nonce is greater than 0.
final var isChildTx = transactionRecord.hasParentConsensusTimestamp() || txId.nonce() > 0;
final var userTxId = isChildTx ? txId.copyBuilder().nonce(0).build() : txId;

// Get or create the history for this transaction ID.
Expand Down
36 changes: 30 additions & 6 deletions ...node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/HandleContextImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.hedera.node.app.spi.HapiUtils.functionOf;
import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.CHILD;
import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.PRECEDING;
import static com.hedera.node.app.workflows.handle.HandleContextImpl.PrecedingTransactionCategory.LIMITED_CHILD_RECORDS;
import static java.util.Objects.requireNonNull;

import com.hedera.hapi.node.base.AccountID;
Expand Down Expand Up @@ -457,7 +458,7 @@ public <T> T dispatchPrecedingTransaction(
@NonNull final Predicate<Key> callback,
@NonNull final AccountID syntheticPayerId) {
final Supplier<SingleTransactionRecordBuilderImpl> recordBuilderFactory =
() -> recordListBuilder.addPreceding(configuration());
() -> recordListBuilder.addPreceding(configuration(), LIMITED_CHILD_RECORDS);
final var result = doDispatchPrecedingTransaction(
syntheticPayerId, txBody, recordBuilderFactory, recordBuilderClass, callback);

Expand All @@ -481,6 +482,19 @@ public <T> T dispatchReversiblePrecedingTransaction(
syntheticPayerId, txBody, recordBuilderFactory, recordBuilderClass, callback);
}

@Override
@NonNull
public <T> T dispatchRemovablePrecedingTransaction(
@NonNull final TransactionBody txBody,
@NonNull final Class<T> recordBuilderClass,
@NonNull final Predicate<Key> callback,
@NonNull final AccountID syntheticPayerId) {
final Supplier<SingleTransactionRecordBuilderImpl> recordBuilderFactory =
() -> recordListBuilder.addRemovablePreceding(configuration());
return doDispatchPrecedingTransaction(
syntheticPayerId, txBody, recordBuilderFactory, recordBuilderClass, callback);
}

@NonNull
public <T> T doDispatchPrecedingTransaction(
@NonNull final AccountID syntheticPayer,
Expand All @@ -495,14 +509,19 @@ public <T> T doDispatchPrecedingTransaction(
if (category != TransactionCategory.USER) {
throw new IllegalArgumentException("Only user-transactions can dispatch preceding transactions");
}

if (stack.depth() > 1) {
throw new IllegalStateException(
"Cannot dispatch a preceding transaction when a savepoint has been created");
}

if (current().isModified()) {
throw new IllegalStateException("Cannot dispatch a preceding transaction when the state has been modified");
}
// This condition fails, because for auto-account creation we charge fees, before dispatching the transaction,
// and the state will be modified.

// if (current().isModified()) {
// throw new IllegalStateException("Cannot dispatch a preceding transaction when the state
// has been modified");
// }

// run the transaction
final var precedingRecordBuilder = recordBuilderFactory.get();
Expand Down Expand Up @@ -643,7 +662,7 @@ private void dispatchSyntheticTxn(
childStack.commitFullStack();
} catch (HandleException e) {
childRecordBuilder.status(e.getStatus());
recordListBuilder.revertChildrenOf(childRecordBuilder);
recordListBuilder.revertChildrenOf(recordBuilder);
}
}

Expand All @@ -657,7 +676,7 @@ public <T> T addChildRecordBuilder(@NonNull final Class<T> recordBuilderClass) {
@Override
@NonNull
public <T> T addPrecedingChildRecordBuilder(@NonNull final Class<T> recordBuilderClass) {
final var result = recordListBuilder.addPreceding(configuration());
final var result = recordListBuilder.addPreceding(configuration(), LIMITED_CHILD_RECORDS);
return castRecordBuilder(result, recordBuilderClass);
}

Expand All @@ -678,4 +697,9 @@ public SavepointStack savepointStack() {
public void revertChildRecords() {
recordListBuilder.revertChildrenOf(recordBuilder);
}

public enum PrecedingTransactionCategory {
UNLIMITED_CHILD_RECORDS,
LIMITED_CHILD_RECORDS
}
}
82 changes: 73 additions & 9 deletions ...ra-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/HandleWorkflow.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
import static com.hedera.hapi.node.base.ResponseCodeEnum.DUPLICATE_TRANSACTION;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_PAYER_SIGNATURE;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_SIGNATURE;
import static com.hedera.hapi.node.base.ResponseCodeEnum.MAX_CHILD_RECORDS_EXCEEDED;
import static com.hedera.hapi.node.base.ResponseCodeEnum.OK;
import static com.hedera.hapi.node.base.ResponseCodeEnum.SUCCESS;
import static com.hedera.node.app.spi.HapiUtils.isHollow;
import static com.hedera.node.app.spi.key.KeyUtils.IMMUTABILITY_SENTINEL_KEY;
import static com.hedera.node.app.state.HederaRecordCache.DuplicateCheckResult.NO_DUPLICATE;
import static com.hedera.node.app.state.HederaRecordCache.DuplicateCheckResult.SAME_NODE;
import static com.hedera.node.app.state.logging.TransactionStateLogger.logStartEvent;
Expand All @@ -44,6 +46,8 @@
import com.hedera.hapi.node.base.ResponseCodeEnum;
import com.hedera.hapi.node.base.SignatureMap;
import com.hedera.hapi.node.base.Transaction;
import com.hedera.hapi.node.state.token.Account;
import com.hedera.hapi.node.token.CryptoUpdateTransactionBody;
import com.hedera.hapi.node.transaction.TransactionBody;
import com.hedera.node.app.fees.ExchangeRateManager;
import com.hedera.node.app.fees.FeeAccumulatorImpl;
Expand All @@ -66,11 +70,13 @@
import com.hedera.node.app.spi.fees.Fees;
import com.hedera.node.app.spi.info.NetworkInfo;
import com.hedera.node.app.spi.info.NodeInfo;
import com.hedera.node.app.spi.workflows.HandleContext;
import com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory;
import com.hedera.node.app.spi.workflows.HandleException;
import com.hedera.node.app.spi.workflows.InsufficientNonFeeDebitsException;
import com.hedera.node.app.spi.workflows.InsufficientServiceFeeException;
import com.hedera.node.app.spi.workflows.PreCheckException;
import com.hedera.node.app.spi.workflows.record.SingleTransactionRecordBuilder;
import com.hedera.node.app.state.HederaRecordCache;
import com.hedera.node.app.state.HederaState;
import com.hedera.node.app.throttle.NetworkUtilizationManager;
Expand All @@ -88,6 +94,7 @@
import com.hedera.node.app.workflows.prehandle.PreHandleWorkflow;
import com.hedera.node.config.ConfigProvider;
import com.hedera.node.config.VersionedConfiguration;
import com.hedera.node.config.data.ConsensusConfig;
import com.hedera.node.config.data.ContractsConfig;
import com.hedera.node.config.data.HederaConfig;
import com.hedera.pbj.runtime.io.buffer.Bytes;
Expand All @@ -102,6 +109,8 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.inject.Inject;
import org.apache.logging.log4j.LogManager;
Expand Down Expand Up @@ -385,6 +394,7 @@ private void handleUserTransaction(
networkUtilizationManager.trackFeePayments(payer, consensusNow, stack);
}
recordBuilder.status(validationResult.responseCodeEnum());

try {
if (validationResult.status() == NODE_DUE_DILIGENCE_FAILURE) {
feeAccumulator.chargeNetworkFee(creator.accountId(), fees.networkFee());
Expand All @@ -408,12 +418,17 @@ private void handleUserTransaction(
}

} else {
networkUtilizationManager.trackTxn(transactionInfo, consensusNow, stack);
if (!authorizer.hasWaivedFees(payer, transactionInfo.functionality(), txBody)) {
// privileged transactions are not charged fees
feeAccumulator.chargeFees(payer, creator.accountId(), fees);
}
try {
// Any hollow accounts that must sign to have all needed signatures, need to be finalized
// as a result of transaction being handled.
finalizeHollowAccounts(context, configuration, preHandleResult.hollowAccounts(), verifier);

networkUtilizationManager.trackTxn(transactionInfo, consensusNow, stack);
if (!authorizer.hasWaivedFees(payer, transactionInfo.functionality(), txBody)) {
// privileged transactions are not charged fees
feeAccumulator.chargeFees(payer, creator.accountId(), fees);
}

if (networkUtilizationManager.wasLastTxnGasThrottled()) {
// Don't charge the payer the service fee component, because the user-submitted transaction
// was fully valid but network capacity was unavailable to satisfy it
Expand Down Expand Up @@ -477,6 +492,53 @@ private void handleUserTransaction(
blockRecordManager.endUserTransaction(recordListResult.records().stream(), state);
}

/**
* Updates key on the hollow accounts that need to be finalized. This is done by dispatching a preceding
* synthetic update transaction. The ksy is derived from the signature expansion, by looking up the ECDSA key
* for the alias.
*
* @param context the handle context
* @param configuration the configuration
* @param accounts the set of hollow accounts that need to be finalized
* @param verifier the key verifier
*/
private void finalizeHollowAccounts(
@NonNull final HandleContext context,
@NonNull final Configuration configuration,
@NonNull final Set<Account> accounts,
@NonNull final DefaultKeyVerifier verifier) {
final var consensusConfig = configuration.getConfigData(ConsensusConfig.class);
final var precedingHollowAccountRecords = accounts.size();
final var maxRecords = consensusConfig.handleMaxPrecedingRecords();
// If the hollow accounts that need to be finalized is greater than the max preceding
// records allowed throw an exception
if (precedingHollowAccountRecords >= maxRecords) {
throw new HandleException(MAX_CHILD_RECORDS_EXCEEDED);
} else {
for (final var hollowAccount : accounts) {
// get the verified key for this hollow account
final var verification = Objects.requireNonNull(
verifier.verificationFor(hollowAccount.alias()),
"Required hollow account verified signature did not exist");
if (verification.key() != null) {
if (!IMMUTABILITY_SENTINEL_KEY.equals(hollowAccount.keyOrThrow())) {
logger.error("Hollow account {} has a key other than the sentinel key", hollowAccount);
return;
}
// dispatch synthetic update transaction for updating key on this hollow account
final var syntheticUpdateTxn = TransactionBody.newBuilder()
.cryptoUpdateAccount(CryptoUpdateTransactionBody.newBuilder()
.accountIDToUpdate(hollowAccount.accountId())
.key(verification.key())
.build())
.build();
context.dispatchPrecedingTransaction(
syntheticUpdateTxn, SingleTransactionRecordBuilder.class, k -> true, context.payer());
}
}
}
}

@NonNull
private FeeAccumulator createFeeAccumulator(
@NonNull final SavepointStackImpl stack,
Expand Down Expand Up @@ -673,8 +735,8 @@ private PreHandleResult addMissingSignatures(

// re-expand keys only if any of the keys have changed
final var previousResults = previousResult.verificationResults();
final var currentRequiredPayerKeys = context.requiredNonPayerKeys();
final var currentOptionalPayerKeys = context.optionalNonPayerKeys();
final var currentRequiredNonPayerKeys = context.requiredNonPayerKeys();
final var currentOptionalNonPayerKeys = context.optionalNonPayerKeys();
final var anyKeyChanged = haveKeyChanges(previousResults, context);
// If none of the keys changed then non need to re-expand all signatures.
if (!anyKeyChanged) {
Expand All @@ -691,9 +753,11 @@ private PreHandleResult addMissingSignatures(
signatureExpander.expand(sigPairs, expanded);
if (payerKey != null && !isHollow(payer)) {
signatureExpander.expand(payerKey, sigPairs, expanded);
} else if (isHollow(payer)) {
context.requireSignatureForHollowAccount(payer);
}
signatureExpander.expand(currentRequiredPayerKeys, sigPairs, expanded);
signatureExpander.expand(currentOptionalPayerKeys, sigPairs, expanded);
signatureExpander.expand(currentRequiredNonPayerKeys, sigPairs, expanded);
signatureExpander.expand(currentOptionalNonPayerKeys, sigPairs, expanded);

// remove all keys that were already verified
for (final var it = expanded.iterator(); it.hasNext(); ) {
Expand Down
11 changes: 10 additions & 1 deletion ...-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/TokenContextImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

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

import static com.hedera.node.app.workflows.handle.HandleContextImpl.PrecedingTransactionCategory.LIMITED_CHILD_RECORDS;
import static com.hedera.node.app.workflows.handle.HandleContextImpl.PrecedingTransactionCategory.UNLIMITED_CHILD_RECORDS;
import static java.util.Objects.requireNonNull;

import com.hedera.node.app.service.token.TokenService;
Expand Down Expand Up @@ -92,7 +94,14 @@ public <T> void forEachChildRecord(@NonNull Class<T> recordBuilderClass, @NonNul
@NonNull
@Override
public <T> T addPrecedingChildRecordBuilder(@NonNull Class<T> recordBuilderClass) {
final var result = recordListBuilder.addPreceding(configuration());
final var result = recordListBuilder.addPreceding(configuration(), LIMITED_CHILD_RECORDS);
return castRecordBuilder(result, recordBuilderClass);
}

@NonNull
@Override
public <T> T addUncheckedPrecedingChildRecordBuilder(@NonNull Class<T> recordBuilderClass) {
final var result = recordListBuilder.addPreceding(configuration(), UNLIMITED_CHILD_RECORDS);
return castRecordBuilder(result, recordBuilderClass);
}

Expand Down
5 changes: 4 additions & 1 deletion ...rc/main/java/com/hedera/node/app/workflows/handle/record/GenesisRecordsConsensusHook.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ private void createAccountRecordBuilders(
.sorted(Comparator.comparingLong(acct -> acct.accountId().accountNum()))
.toList();
for (final Account key : orderedAccts) {
final var recordBuilder = context.addPrecedingChildRecordBuilder(GenesisAccountRecordBuilder.class);
// we create preceding records on genesis for each system account created.
// This is an exception and should not fail with MAX_CHILD_RECORDS_EXCEEDED
final var recordBuilder =
context.addUncheckedPrecedingChildRecordBuilder(GenesisAccountRecordBuilder.class);
final var accountId = requireNonNull(key.accountId());
recordBuilder.accountID(accountId);
if (recordMemo != null) {
Expand Down
Loading

0 comments on commit 20bdd4a

Please sign in to comment.