Skip to content

Commit

Permalink
chore: standardize and simplify ScheduleService (#16053)
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Tinker <[email protected]>
  • Loading branch information
tinker-michaelj authored Oct 22, 2024
1 parent 11f878c commit f1c9cf6
Show file tree
Hide file tree
Showing 35 changed files with 745 additions and 1,813 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public interface FeeContext {
*
* @return the {@code Configuration}
*/
@Nullable
@NonNull
Configuration configuration();

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@
* <br>These include maps, sets, lists, arrays, etc...
* <br>The methods in this class are used in hot spot code, so allocation must be kept to a bare
* minimum, and anything likely to have performance questions should be avoided.
* <br>Note that comparing keys is unavoidably costly. We try to exit as early as possible throughout
* this class, but worst case we're comparing every simple key byte-by-byte for the entire tree, which
* may be up to 15 levels deep with any number of keys per level. We haven't seen a key with
* several million "simple" keys included, but that does not mean nobody will create one.
* <br>Note that comparing keys can be fairly costly, as in principle a key structure can have a
* serialized size up to about {@code TransactionConfig#transactionMaxBytes()}. We try to exit as
* early as possible throughout this class, but worst case we're comparing every simple key
* byte-by-byte for the entire tree.
*/
public class KeyComparator implements Comparator<Key> {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,20 @@

package com.hedera.node.app.spi.key;

import static java.util.Collections.unmodifiableSortedSet;

import com.hedera.hapi.node.base.Key;
import com.hedera.node.app.spi.signatures.SignatureVerification;
import com.hedera.node.app.spi.signatures.VerificationAssistant;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.util.SortedSet;
import java.util.TreeSet;

/**
* Helper class that contains all functionality for verifying signatures during handle.
*/
public interface KeyVerifier {
SortedSet<Key> NO_CRYPTO_KEYS = unmodifiableSortedSet(new TreeSet<>(new KeyComparator()));

/**
* Gets the {@link SignatureVerification} for the given key. If this key was not provided during pre-handle, then
Expand Down Expand Up @@ -60,4 +65,20 @@ public interface KeyVerifier {
*/
@NonNull
SignatureVerification verificationFor(@NonNull Key key, @NonNull VerificationAssistant callback);

/**
* <b>If</b> this verifier is based on cryptographic verification of signatures on a transaction submitted from
* outside the blockchain, returns the set of cryptographic keys that had valid signatures, ordered by the
* {@link KeyComparator}.
* <p>
* Default is an empty set, for verifiers that use a more abstract concept of signing, such as,
* <ol>
* <li>Whether a key references the contract whose EVM address is the recipient address of the active frame.</li>
* <li>Whether a key is present in the signatories list of a scheduled transaction.</li>
* </ol>
* @return the set of cryptographic keys that had valid signatures for this transaction.
*/
default SortedSet<Key> signingCryptoKeys() {
return NO_CRYPTO_KEYS;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -276,13 +276,13 @@ PreHandleContext requireKeyIfReceiverSigRequired(
/**
* Returns all (required and optional) keys of a nested transaction.
*
* @param nestedTxn the {@link TransactionBody} which keys are needed
* @param payerForNested the payer for the nested transaction
* @param body the {@link TransactionBody} which keys are needed
* @param payerId the payer for the nested transaction
* @return the set of keys
* @throws PreCheckException If there is a problem with the nested transaction
*/
@NonNull
TransactionKeys allKeysForTransaction(@NonNull TransactionBody nestedTxn, @NonNull AccountID payerForNested)
TransactionKeys allKeysForTransaction(@NonNull TransactionBody body, @NonNull AccountID payerId)
throws PreCheckException;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
* Contains all keys and hollow accounts (required and optional) of a transaction.
*/
public interface TransactionKeys {

/**
* Getter for the payer key
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,7 @@ public PreHandleContext requireSignatureForHollowAccountCreation(@NonNull final

@NonNull
@Override
public TransactionKeys allKeysForTransaction(
@NonNull TransactionBody nestedTxn, @NonNull AccountID payerForNested) {
public TransactionKeys allKeysForTransaction(@NonNull TransactionBody body, @NonNull AccountID payerId) {
throw new UnsupportedOperationException("Not yet implemented");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public FeeCalculatorFactory feeCalculatorFactory() {
}

@Override
public @Nullable Configuration configuration() {
public @NonNull Configuration configuration() {
return context.configuration();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,36 @@

import com.hedera.hapi.node.base.Key;
import com.hedera.hapi.node.base.KeyList;
import com.hedera.node.app.spi.key.KeyComparator;
import com.hedera.node.app.spi.signatures.SignatureVerification;
import com.hedera.node.app.spi.signatures.VerificationAssistant;
import com.hedera.node.config.data.HederaConfig;
import com.hedera.pbj.runtime.io.buffer.Bytes;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;
import java.util.AbstractMap;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

/**
* Base implementation of {@link AppKeyVerifier}
*/
public class DefaultKeyVerifier implements AppKeyVerifier {

private static final Logger logger = LogManager.getLogger(DefaultKeyVerifier.class);

private static final Comparator<Key> KEY_COMPARATOR = new KeyComparator();

private final int legacyFeeCalcNetworkVpt;
private final long timeout;
private final Map<Key, SignatureVerificationFuture> keyVerifications;
Expand Down Expand Up @@ -137,6 +144,16 @@ public int numSignaturesVerified() {
return legacyFeeCalcNetworkVpt;
}

@Override
public SortedSet<Key> signingCryptoKeys() {
return keyVerifications.entrySet().stream()
.map(entry -> new AbstractMap.SimpleImmutableEntry<>(
entry.getKey(), resolveFuture(entry.getValue(), () -> failedVerification(entry.getKey()))))
.filter(e -> e.getValue().passed())
.map(Map.Entry::getKey)
.collect(Collectors.toCollection(() -> new TreeSet<>(KEY_COMPARATOR)));
}

/**
* Get a {@link Future<SignatureVerification>} for the given key.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,8 @@ public boolean tryToChargePayer(final long amount) {
return feeAccumulator.chargeNetworkFee(payerId, amount);
}

@NonNull
@Override
public Configuration configuration() {
public @NonNull Configuration configuration() {
return config;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

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

import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_PAYER_ACCOUNT_ID;
import static com.hedera.hapi.node.base.ResponseCodeEnum.UNRESOLVABLE_REQUIRED_SIGNERS;
import static com.hedera.hapi.util.HapiUtils.EMPTY_KEY_LIST;
import static com.hedera.hapi.util.HapiUtils.isHollow;
import static com.hedera.node.app.service.token.impl.util.TokenHandlerHelper.verifyNotEmptyKey;
Expand Down Expand Up @@ -62,7 +64,7 @@ public class PreHandleContextImpl implements PreHandleContext {
/**
* The payer account ID. Specified in the transaction body, extracted and stored separately for convenience.
*/
private final AccountID payer;
private final AccountID payerId;
/**
* The payer's key, as found in state
*/
Expand Down Expand Up @@ -128,32 +130,28 @@ public PreHandleContextImpl(
}

/**
* Create a new instance
* Create a new instance of {@link PreHandleContextImpl}.
* @throws PreCheckException if the payer account does not exist
*/
private PreHandleContextImpl(
@NonNull final ReadableStoreFactory storeFactory,
@NonNull final TransactionBody txn,
@NonNull final AccountID payer,
@NonNull final AccountID payerId,
@NonNull final Configuration configuration,
@NonNull final TransactionDispatcher dispatcher,
final boolean isUserTx)
throws PreCheckException {
this.storeFactory = requireNonNull(storeFactory, "storeFactory must not be null.");
this.txn = requireNonNull(txn, "txn must not be null!");
this.payer = requireNonNull(payer, "payer msut not be null!");
this.payerId = requireNonNull(payerId, "payer must not be null!");
this.configuration = requireNonNull(configuration, "configuration must not be null!");
this.dispatcher = requireNonNull(dispatcher, "dispatcher must not be null!");
this.isUserTx = isUserTx;

this.accountStore = storeFactory.getStore(ReadableAccountStore.class);

// Find the account, which must exist or throw a PreCheckException with the given response code.
final var account = accountStore.getAccountById(payer);
mustExist(account, ResponseCodeEnum.INVALID_PAYER_ACCOUNT_ID);
// NOTE: While it is true that the key can be null on some special accounts like
// account 800, those accounts cannot be the payer.
payerKey = account.key();
mustExist(payerKey, ResponseCodeEnum.INVALID_PAYER_ACCOUNT_ID);
// Find the account, which must exist or throw on construction
final var payer = mustExist(accountStore.getAccountById(payerId), INVALID_PAYER_ACCOUNT_ID);
// It would be a catastrophic invariant failure if an account in state didn't have a key
payerKey = payer.keyOrThrow();
}

@Override
Expand All @@ -171,7 +169,7 @@ public TransactionBody body() {
@Override
@NonNull
public AccountID payer() {
return payer;
return payerId;
}

@Override
Expand Down Expand Up @@ -310,7 +308,7 @@ public PreHandleContext requireAliasedKeyOrThrow(
// If we repeated the payer requirement, we would be requiring "double authorization" from
// the contract doing the dispatch; but the contract has already authorized the action by
// the very execution of its bytecode.
if (accountID.equals(payer)) {
if (accountID.equals(payerId)) {
return this;
}
final Account account;
Expand All @@ -330,7 +328,7 @@ public PreHandleContext requireAliasedKeyOrThrow(
}
// Verify this key isn't for an immutable account
verifyNotStakingAccounts(account.accountIdOrThrow(), responseCode);
final var key = account.key();
final var key = account.keyOrThrow();
if (!isValid(key)) { // Or if it is a Contract Key? Or if it is an empty key?
// Or a KeyList with no
// keys? Or KeyList with Contract keys only?
Expand Down Expand Up @@ -478,18 +476,20 @@ public PreHandleContext requireSignatureForHollowAccountCreation(@NonNull final

@NonNull
@Override
public TransactionKeys allKeysForTransaction(
@NonNull TransactionBody nestedTxn, @NonNull final AccountID payerForNested) throws PreCheckException {
dispatcher.dispatchPureChecks(nestedTxn);
final var nestedContext =
new PreHandleContextImpl(storeFactory, nestedTxn, payerForNested, configuration, dispatcher);
public TransactionKeys allKeysForTransaction(@NonNull TransactionBody body, @NonNull final AccountID payerId)
throws PreCheckException {
// Throws PreCheckException if the transaction body is structurally invalid
dispatcher.dispatchPureChecks(body);
// Throws PreCheckException if the payer account does not exist
final var context = new PreHandleContextImpl(storeFactory, body, payerId, configuration, dispatcher);
try {
dispatcher.dispatchPreHandle(nestedContext);
// Accumulate all required keys in the context
dispatcher.dispatchPreHandle(context);
} catch (final PreCheckException ignored) {
// We must ignore/translate the exception here, as this is key gathering, not transaction validation.
throw new PreCheckException(ResponseCodeEnum.UNRESOLVABLE_REQUIRED_SIGNERS);
// Translate all prehandle failures to unresolvable required signers
throw new PreCheckException(UNRESOLVABLE_REQUIRED_SIGNERS);
}
return nestedContext;
return context;
}

@Override
Expand All @@ -512,8 +512,8 @@ public PreHandleContext innerContext() {
public String toString() {
return "PreHandleContextImpl{" + "accountStore="
+ accountStore + ", txn="
+ txn + ", payer="
+ payer + ", payerKey="
+ txn + ", payerId="
+ payerId + ", payerKey="
+ payerKey + ", requiredNonPayerKeys="
+ requiredNonPayerKeys + ", innerContext="
+ innerContext + ", storeFactory="
Expand Down
Loading

0 comments on commit f1c9cf6

Please sign in to comment.