Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Signed-off-by: Fabio Di Fabio <[email protected]>
  • Loading branch information
fab-10 committed Jun 21, 2024
1 parent b8b6f85 commit b3ca959
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,24 @@
import java.io.IOException;
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import linea.plugin.acc.test.tests.web3j.generated.SimpleStorage;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.apache.tuweni.units.bigints.UInt32;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration;
Expand All @@ -38,6 +46,10 @@
import org.hyperledger.besu.tests.acceptance.dsl.account.Accounts;
import org.hyperledger.besu.tests.acceptance.dsl.condition.txpool.TxPoolConditions;
import org.hyperledger.besu.tests.acceptance.dsl.node.BesuNode;
import org.hyperledger.besu.tests.acceptance.dsl.node.RunnableNode;
import org.hyperledger.besu.tests.acceptance.dsl.node.configuration.BesuNodeConfigurationBuilder;
import org.hyperledger.besu.tests.acceptance.dsl.node.configuration.NodeConfigurationFactory;
import org.hyperledger.besu.tests.acceptance.dsl.node.configuration.genesis.GenesisConfigurationFactory;
import org.hyperledger.besu.tests.acceptance.dsl.node.configuration.genesis.GenesisConfigurationFactory.CliqueOptions;
import org.hyperledger.besu.tests.acceptance.dsl.transaction.txpool.TxPoolTransactions;
import org.junit.jupiter.api.AfterEach;
Expand Down Expand Up @@ -66,7 +78,7 @@ public class LineaPluginTestBase extends AcceptanceTestBase {
@BeforeEach
public void setup() throws Exception {
minerNode =
besu.createCliqueNodeWithExtraCliOptionsAndRpcApis(
createCliqueNodeWithExtraCliOptionsAndRpcApis(
"miner1", LINEA_CLIQUE_OPTIONS, getTestCliOptions(), Set.of("LINEA", "MINER"));
minerNode.setTransactionPoolConfiguration(
ImmutableTransactionPoolConfiguration.builder()
Expand All @@ -86,6 +98,58 @@ public void stop() {
cluster.close();
}

protected Optional<Bytes32> maybeCustomGenesisExtraData() {
return Optional.empty();
}

private BesuNode createCliqueNodeWithExtraCliOptionsAndRpcApis(
final String name,
final CliqueOptions cliqueOptions,
final List<String> extraCliOptions,
final Set<String> extraRpcApis)
throws IOException {
final NodeConfigurationFactory node = new NodeConfigurationFactory();

final var nodeConfBuilder =
new BesuNodeConfigurationBuilder()
.name(name)
.miningEnabled()
.jsonRpcConfiguration(node.createJsonRpcWithCliqueEnabledConfig(extraRpcApis))
.webSocketConfiguration(node.createWebSocketEnabledConfig())
.inProcessRpcConfiguration(node.createInProcessRpcConfiguration(extraRpcApis))
.devMode(false)
.jsonRpcTxPool()
.genesisConfigProvider(
validators -> Optional.of(provideGenesisConfig(validators, cliqueOptions)))
.extraCLIOptions(extraCliOptions);

return besu.create(nodeConfBuilder.build());
}

private String provideGenesisConfig(
final Collection<? extends RunnableNode> validators, final CliqueOptions cliqueOptions) {
final var genesis =
GenesisConfigurationFactory.createCliqueGenesisConfig(validators, cliqueOptions).get();

return maybeCustomGenesisExtraData()
.map(ed -> setGenesisCustomExtraData(genesis, ed))
.orElse(genesis);
}

private String setGenesisCustomExtraData(final String genesis, final Bytes32 customExtraData) {
final var om = new ObjectMapper();
final ObjectNode root;
try {
root = (ObjectNode) om.readTree(genesis);
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
final var existingExtraData = Bytes.fromHexString(root.get("extraData").asText());
final var updatedExtraData = Bytes.concatenate(customExtraData, existingExtraData.slice(32));
root.put("extraData", updatedExtraData.toHexString());
return root.toPrettyString();
}

protected void sendTransactionsWithGivenLengthPayload(
final SimpleStorage simpleStorage,
final List<String> accounts,
Expand Down Expand Up @@ -225,4 +289,14 @@ protected String sendTransactionWithGivenLengthPayload(
BigInteger.ZERO)
.getTransactionHash();
}

protected Bytes32 createExtraDataPricingField(
final long fixedCostKWei, final long variableCostKWei, final long minGasPriceKWei) {
final UInt32 fixed = UInt32.valueOf(BigInteger.valueOf(fixedCostKWei));
final UInt32 variable = UInt32.valueOf(BigInteger.valueOf(variableCostKWei));
final UInt32 min = UInt32.valueOf(BigInteger.valueOf(minGasPriceKWei));

return Bytes32.rightPad(
Bytes.concatenate(Bytes.of((byte) 1), fixed.toBytes(), variable.toBytes(), min.toBytes()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@

import linea.plugin.acc.test.LineaPluginTestBase;
import linea.plugin.acc.test.TestCommandLineOptionsBuilder;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.apache.tuweni.units.bigints.UInt32;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.tests.acceptance.dsl.account.Account;
import org.hyperledger.besu.tests.acceptance.dsl.account.Accounts;
Expand Down Expand Up @@ -62,7 +60,7 @@ public void updateMinGasPriceViaExtraData() {
final var extraData =
createExtraDataPricingField(
0, MIN_GAS_PRICE.toLong() / WEI_IN_KWEI, doubleMinGasPrice.toLong() / WEI_IN_KWEI);
final var reqSetExtraData = new ExtraDataPricingTest.MinerSetExtraDataRequest(extraData);
final var reqSetExtraData = new MinerSetExtraDataRequest(extraData);
final var respSetExtraData = reqSetExtraData.execute(minerNode.nodeRequests());

assertThat(respSetExtraData).isTrue();
Expand Down Expand Up @@ -99,9 +97,9 @@ public void updateProfitabilityParamsViaExtraData() throws IOException {
// when this first tx is mined the above extra data pricing will have effect on following txs
final TransferTransaction profitableTx =
accountTransactions.createTransfer(sender, recipient, 1);
final var protitableTx = minerNode.execute(profitableTx);
final var profitableTxHash = minerNode.execute(profitableTx);

minerNode.verify(eth.expectSuccessfulTransactionReceipt(protitableTx.toHexString()));
minerNode.verify(eth.expectSuccessfulTransactionReceipt(profitableTxHash.toHexString()));

// this tx will be evaluated with the previously set extra data pricing to be unprofitable
final RawTransaction unprofitableTx =
Expand Down Expand Up @@ -149,14 +147,4 @@ public Boolean execute(final NodeRequests nodeRequests) {

static class MinerSetExtraDataResponse extends org.web3j.protocol.core.Response<Boolean> {}
}

private Bytes32 createExtraDataPricingField(
final long fixedCostKWei, final long variableCostKWei, final long minGasPriceKWei) {
final UInt32 fixed = UInt32.valueOf(BigInteger.valueOf(fixedCostKWei));
final UInt32 variable = UInt32.valueOf(BigInteger.valueOf(variableCostKWei));
final UInt32 min = UInt32.valueOf(BigInteger.valueOf(minGasPriceKWei));

return Bytes32.rightPad(
Bytes.concatenate(Bytes.of((byte) 1), fixed.toBytes(), variable.toBytes(), min.toBytes()));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright Consensys Software Inc.
*
* 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.
*
* SPDX-License-Identifier: Apache-2.0
*/
package linea.plugin.acc.test.extradata;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.List;
import java.util.Optional;

import linea.plugin.acc.test.LineaPluginTestBase;
import linea.plugin.acc.test.TestCommandLineOptionsBuilder;
import org.apache.tuweni.bytes.Bytes32;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.tests.acceptance.dsl.account.Account;
import org.hyperledger.besu.tests.acceptance.dsl.transaction.account.TransferTransaction;
import org.junit.jupiter.api.Test;

public class StartupExtraDataPricingTest extends LineaPluginTestBase {
private static final Wei VARIABLE_GAS_COST = Wei.of(1_200_300_000);
private static final Wei MIN_GAS_PRICE = VARIABLE_GAS_COST.divide(2);
private static final int WEI_IN_KWEI = 1000;

@Override
public List<String> getTestCliOptions() {
return getTestCommandLineOptionsBuilder().build();
}

protected TestCommandLineOptionsBuilder getTestCommandLineOptionsBuilder() {
return new TestCommandLineOptionsBuilder()
.set("--plugin-linea-extra-data-pricing-enabled=", Boolean.TRUE.toString());
}

@Override
protected Optional<Bytes32> maybeCustomGenesisExtraData() {
final var genesisExtraData =
createExtraDataPricingField(
0, VARIABLE_GAS_COST.toLong() / WEI_IN_KWEI, MIN_GAS_PRICE.toLong() / WEI_IN_KWEI);

return Optional.of(genesisExtraData);
}

@Test
public void minGasPriceSetFromChainHeadExtraDataAtStartup() {
// at startup the min gas price should be set from the current chain head block extra data
assertThat(minerNode.getMiningParameters().getMinTransactionGasPrice())
.isEqualTo(MIN_GAS_PRICE);

final Account sender = accounts.getSecondaryBenefactor();
final Account recipient = accounts.createAccount("recipient");

final TransferTransaction transferTx = accountTransactions.createTransfer(sender, recipient, 1);
final var txHash = minerNode.execute(transferTx);

minerNode.verify(eth.expectSuccessfulTransactionReceipt(txHash.toHexString()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.web3j.crypto.Credentials;
import org.web3j.protocol.Web3j;
import org.web3j.protocol.core.Request;
import org.web3j.protocol.core.Response;
import org.web3j.protocol.http.HttpService;
import org.web3j.tx.RawTransactionManager;
import org.web3j.tx.TransactionManager;
Expand Down Expand Up @@ -174,20 +175,20 @@ public void successfulUpdatePricingParameters() throws IOException {
minerNode.verify(eth.expectNoTransactionReceipt(txUnprofitable.getTransactionHash()));

final var zeroFixedCostKWei = "00000000";
final var minimalVaribleCostKWei = "00000001";
final var minimalMinGasPriceKWei = "00000001";
final var minimalVariableCostKWei = "00000001";
final var minimalMinGasPriceKWei = "00000002";
final var extraData =
Bytes32.fromHexString(
"0x01"
+ zeroFixedCostKWei
+ minimalVaribleCostKWei
+ minimalVariableCostKWei
+ minimalMinGasPriceKWei
+ "00000000000000000000000000000000000000");

final var reqLinea = new LineaSetExtraDataRequest(extraData);
final var respLinea = reqLinea.execute(minerNode.nodeRequests());
assertThat(respLinea).isTrue();
assertThat(minerNode.getMiningParameters().getMinTransactionGasPrice()).isEqualTo(Wei.of(1000));
assertThat(minerNode.getMiningParameters().getMinTransactionGasPrice()).isEqualTo(Wei.of(2000));
// assert that tx is confirmed now
minerNode.verify(eth.expectSuccessfulTransactionReceipt(txUnprofitable.getTransactionHash()));
}
Expand Down Expand Up @@ -236,16 +237,15 @@ public Boolean execute(final NodeRequests nodeRequests) {
}
}

static class FailingLineaSetExtraDataRequest
implements Transaction<org.web3j.protocol.core.Response.Error> {
static class FailingLineaSetExtraDataRequest implements Transaction<Response.Error> {
private final Bytes extraData;

public FailingLineaSetExtraDataRequest(final Bytes extraData) {
this.extraData = extraData;
}

@Override
public org.web3j.protocol.core.Response.Error execute(final NodeRequests nodeRequests) {
public Response.Error execute(final NodeRequests nodeRequests) {
try {
return new Request<>(
"linea_setExtraData",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import java.util.concurrent.atomic.AtomicInteger;

import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.extern.slf4j.Slf4j;
import net.consensys.linea.extradata.LineaExtraDataException;
import net.consensys.linea.extradata.LineaExtraDataHandler;
Expand Down Expand Up @@ -58,17 +57,15 @@ public String getName() {
}

public Boolean execute(final PluginRpcRequest request) {
if (log.isDebugEnabled()) {
// no matter if it overflows, since it is only used to correlate logs for this request,
// so we only print callParameters once at the beginning, and we can reference them using the
// sequence.
LOG_SEQUENCE.incrementAndGet();
}
// no matter if it overflows, since it is only used to correlate logs for this request,
// so we only print callParameters once at the beginning, and we can reference them using the
// sequence.
final int logId = log.isDebugEnabled() ? LOG_SEQUENCE.incrementAndGet() : -1;

try {
final var extraData = parseRequest(request.getParams());
final var extraData = parseRequest(logId, request.getParams());

updatePricingConf(extraData);
updatePricingConf(logId, extraData);

updateStandardExtraData(extraData);

Expand All @@ -87,21 +84,21 @@ private void updateStandardExtraData(final Bytes32 extraData) {
}
}

private void updatePricingConf(final Bytes32 extraData) {
private void updatePricingConf(final int logId, final Bytes32 extraData) {
extraDataHandler.handle(extraData);
log.atDebug()
.setMessage("[{}] Successfully handled extra data pricing")
.addArgument(LOG_SEQUENCE::get)
.addArgument(logId)
.log();
}

private Bytes32 parseRequest(final Object[] params) {
private Bytes32 parseRequest(final int logId, final Object[] params) {
try {
final var rawParam = parameterParser.required(params, 0, String.class);
final var extraData = Bytes32.wrap(Bytes.fromHexStringLenient(rawParam));
log.atDebug()
.setMessage("[{}] set extra data, raw=[{}] parsed=[{}]")
.addArgument(LOG_SEQUENCE::get)
.addArgument(logId)
.addArgument(rawParam)
.addArgument(extraData::toHexString)
.log();
Expand All @@ -111,11 +108,6 @@ private Bytes32 parseRequest(final Object[] params) {
}
}

public record Response(
@JsonProperty Boolean gasLimit,
@JsonProperty String baseFeePerGas,
@JsonProperty String priorityFeePerGas) {}

private record ExtraDataPricingError(LineaExtraDataException ex) implements RpcMethodError {
@Override
public int getCode() {
Expand Down

0 comments on commit b3ca959

Please sign in to comment.