Skip to content
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.

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

Corrected eth_getLogs default fromBlock value. #4513

Merged
merged 8 commits into from
Oct 26, 2022
Prev Previous commit
Next Next commit
Refactoring. Tests.
Signed-off-by: mark-terry <mark.terry@consensys.net>
  • Loading branch information
mark-terry committed Oct 18, 2022
commit 047819569860707f01f1ac62a2594531b6bab765
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.BlockParameter;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.FilterParameter;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
Expand Down Expand Up @@ -70,8 +69,8 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
final long fromBlockNumber;
final long toBlockNumber;
try {
fromBlockNumber = interpretBlockParam(filter.getFromBlock());
toBlockNumber = interpretBlockParam(filter.getToBlock());
fromBlockNumber = filter.getFromBlock().getBlockNumber(blockchain);
toBlockNumber = filter.getToBlock().getBlockNumber(blockchain);
} catch (final Exception e) {
ex.set(e);
return Collections.emptyList();
Expand All @@ -93,26 +92,4 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
return new JsonRpcSuccessResponse(
requestContext.getRequest().getId(), new LogsResult(matchingLogs));
}

private long interpretBlockParam(final BlockParameter block) throws Exception {
if (block.isFinalized()) {
return blockchain
.finalizedBlockHeader()
.orElseThrow(() -> new Exception("Finalized block not found."))
.getNumber();
} else if (block.isLatest()) {
return blockchain.headBlockNumber();
} else if (block.isPending()) {
// Pending not implemented, returns latest
return blockchain.headBlockNumber();
} else if (block.isSafe()) {
return blockchain
.safeBlockHeader()
.orElseThrow(() -> new Exception("Safe block not found."))
.getNumber();
} else {
// Alternate cases (numeric input or "earliest")
return block.getNumber().get();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters;

import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
import org.hyperledger.besu.ethereum.core.BlockHeader;

import java.util.Objects;
Expand Down Expand Up @@ -99,6 +100,28 @@ public boolean isNumeric() {
return this.type == BlockParameterType.NUMERIC;
}

public long getBlockNumber(final BlockchainQueries blockchain) throws Exception {
if (this.isFinalized()) {
return blockchain
.finalizedBlockHeader()
.orElseThrow(() -> new Exception("Finalized block not found."))
.getNumber();
} else if (this.isLatest()) {
return blockchain.headBlockNumber();
} else if (this.isPending()) {
// Pending not implemented, returns latest
return blockchain.headBlockNumber();
} else if (this.isSafe()) {
return blockchain
.safeBlockHeader()
.orElseThrow(() -> new Exception("Safe block not found."))
.getNumber();
} else {
// Alternate cases (numeric input or "earliest")
return this.getNumber().get();
}
}

@Override
public String toString() {
return "BlockParameter{" + "type=" + type + ", number=" + number + '}';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -228,7 +229,7 @@ public void shouldQueryEarliestToLatest() {
public void shouldQuerySafeToFinalized() {
final long finalizedBlockNumber = 50L;
final long safeBlockNumber = 25L;
final JsonRpcRequestContext request = buildRequest("safe", "Finalized");
final JsonRpcRequestContext request = buildRequest("safe", "finalized");

final BlockHeader finalizedBlockHeader = mock(BlockHeader.class);
final BlockHeader safeBlockHeader = mock(BlockHeader.class);
Expand All @@ -245,6 +246,8 @@ public void shouldQuerySafeToFinalized() {

verify(blockchainQueries)
.matchingLogs(eq(safeBlockNumber), eq(finalizedBlockNumber), any(), any());
verify(blockchainQueries, times(1)).finalizedBlockHeader();
verify(blockchainQueries, times(1)).safeBlockHeader();
verify(blockchainQueries, never()).headBlockNumber();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,30 @@ public void longShouldReturnLongNumberValue() {
assertThat(blockParameter.isSafe()).isFalse();
}

@Test
public void upperCaseStringShouldBeHandled() {
final BlockParameter blockParameter = new BlockParameter("LATEST");
assertThat(blockParameter.getNumber()).isEmpty();
assertThat(blockParameter.isLatest()).isTrue();
assertThat(blockParameter.isEarliest()).isFalse();
assertThat(blockParameter.isFinalized()).isFalse();
assertThat(blockParameter.isNumeric()).isFalse();
assertThat(blockParameter.isPending()).isFalse();
assertThat(blockParameter.isSafe()).isFalse();
}

@Test
public void mixedCaseStringShouldBeHandled() {
final BlockParameter blockParameter = new BlockParameter("lATest");
assertThat(blockParameter.getNumber()).isEmpty();
assertThat(blockParameter.isLatest()).isTrue();
assertThat(blockParameter.isEarliest()).isFalse();
assertThat(blockParameter.isFinalized()).isFalse();
assertThat(blockParameter.isNumeric()).isFalse();
assertThat(blockParameter.isPending()).isFalse();
assertThat(blockParameter.isSafe()).isFalse();
}

@Test
public void invalidValueShouldThrowException() {
assertThatThrownBy(() -> new BlockParameter("invalid"))
Expand Down