Skip to content

Precompute authorities when importing blocks #8017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fab-10 merged 2 commits into from
Dec 16, 2024
Merged

Precompute authorities when importing blocks #8017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d98207f
Precompute authorities when importing blocks
fab-10 Dec 11, 2024
63dbb19
Using Supplier to make the authorizer thread safe
fab-10 Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 43 additions & 15 deletions ...perledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.WithdrawalsValidatorProvider.getWithdrawalsValidator;

import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.BlobGas;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.datatypes.RequestType;
Expand Down Expand Up @@ -222,20 +221,8 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
blockParam.getTransactions().stream()
.map(Bytes::fromHexString)
.map(in -> TransactionDecoder.decodeOpaqueBytes(in, EncodingContext.BLOCK_BODY))
.collect(Collectors.toList());
transactions.forEach(
transaction ->
mergeCoordinator
.getEthScheduler()
.scheduleTxWorkerTask(
() -> {
Address sender = transaction.getSender();
LOG.atTrace()
.setMessage("The sender for transaction {} is calculated : {}")
.addArgument(transaction::getHash)
.addArgument(sender)
.log();
}));
.toList();
precomputeSenders(transactions);
} catch (final RLPException | IllegalArgumentException e) {
return respondWithInvalid(
reqId,
Expand Down Expand Up @@ -392,6 +379,47 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
}
}

private void precomputeSenders(final List<Transaction> transactions) {
transactions.forEach(
transaction -> {
mergeCoordinator
.getEthScheduler()
.scheduleTxWorkerTask(
() -> {
final var sender = transaction.getSender();
LOG.atTrace()
.setMessage("The sender for transaction {} is calculated : {}")
.addArgument(transaction::getHash)
.addArgument(sender)
.log();
});
if (transaction.getType().supportsDelegateCode()) {
precomputeAuthorities(transaction);
}
});
}

private void precomputeAuthorities(final Transaction transaction) {
final var codeDelegations = transaction.getCodeDelegationList().get();
int index = 0;
for (final var codeDelegation : codeDelegations) {
final var constIndex = index++;
mergeCoordinator
.getEthScheduler()
.scheduleTxWorkerTask(
Copy link
Copy Markdown
Contributor

@ahamlat ahamlat Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Precompute is asynchronous here, are we sure that the authorities are not accessed before it is finished or there is a fallback to calculate them if they don't exist yet ?

Copy link
Copy Markdown
Contributor

@ahamlat ahamlat Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it is not better to precalculate all the senders than precompute all the autorities of all the transactions. Currently, we calculate sender of txn, then precomupte autorities of txn, then calculate sender of txn+1, then precomupte autorities of txn+1

Copy link
Copy Markdown
Contributor Author

@fab-10 fab-10 Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

about your first question, if this async computation does not finish before the processing, then like for the sender it will be computed on first usage

About the order, I do not know which is the best solution, because in serial mode, should be better to precompute the authorities to txn before the sender of txn+1, but in parallel mode not sure what is the best solution, what do you suggest?

Copy link
Copy Markdown
Contributor

@ahamlat ahamlat Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about this case, do we want to improve the time it takes to execute all the transaction by precalculating all the sender or improve a large one by precalculating all the authorizers.
I think it makes sens here to improve the worst case, so let's keep the current order.

() -> {
final var authority = codeDelegation.authorizer();
LOG.atTrace()
.setMessage(
"The code delegation authority at index {} for transaction {} is calculated : {}")
.addArgument(constIndex)
.addArgument(transaction::getHash)
.addArgument(authority)
.log();
});
}
}

JsonRpcResponse respondWith(
final Object requestId,
final EnginePayloadParameter param,
Expand Down
11 changes: 3 additions & 8 deletions ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/CodeDelegation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ public class CodeDelegation implements org.hyperledger.besu.datatypes.CodeDelega
private final Address address;
private final long nonce;
private final SECPSignature signature;
private Optional<Address> authorizer = Optional.empty();
private boolean isAuthorityComputed = false;
private final Supplier<Optional<Address>> authorizerSupplier =
Suppliers.memoize(this::computeAuthority);

/**
* An access list entry as defined in EIP-7702
Expand Down Expand Up @@ -107,12 +107,7 @@ public SECPSignature signature() {

@Override
public Optional<Address> authorizer() {
if (!isAuthorityComputed) {
authorizer = computeAuthority();
isAuthorityComputed = true;
}

return authorizer;
return authorizerSupplier.get();
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion .../eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ public interface MemorySize {
int ACCESS_LIST_ENTRY_SHALLOW_SIZE = 248;
int OPTIONAL_ACCESS_LIST_SHALLOW_SIZE = 40;
int OPTIONAL_CODE_DELEGATION_LIST_SHALLOW_SIZE = 40;
int CODE_DELEGATION_ENTRY_SIZE = 432;
int CODE_DELEGATION_ENTRY_SIZE = 472;
int VERSIONED_HASH_SIZE = 96;
int LIST_SHALLOW_SIZE = 48;
int OPTIONAL_SHALLOW_SIZE = 16;
Expand Down