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

Adding min and max ranges to check the value provided by the Oracle's #2922

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
fmacleal marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ public class StableMinGasPriceSystemConfig {
private static final String METHOD_PROPERTY = "source.method";
private static final String PARAMS_PROPERTY = "source.params";

private static final String PARAM_MIN_VALID_PRICE = "minValidPrice";
private static final String PARAM_MAX_VALID_PRICE = "maxValidPrice";

private final Duration refreshRate;
private final Long minStableGasPrice;
private final boolean enabled;
Expand Down Expand Up @@ -68,4 +71,13 @@ public EthCallMinGasPriceSystemConfig getEthCallConfig() {
public MinGasPriceProviderType getMethod() {
return method;
}

public Long getMinValidPrice() {
return config.hasPath(PARAM_MIN_VALID_PRICE) ? config.getLong(PARAM_MIN_VALID_PRICE) : 0;
}

public Long getMaxValidPrice() {
return config.hasPath(PARAM_MAX_VALID_PRICE) ? config.getLong(PARAM_MAX_VALID_PRICE) : Long.MAX_VALUE;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class EthCallMinGasPriceProvider extends StableMinGasPriceProvider {
private final Supplier<EthModule> ethModuleSupplier;

public EthCallMinGasPriceProvider(MinGasPriceProvider fallBackProvider, StableMinGasPriceSystemConfig config, Supplier<EthModule> ethModuleSupplier) {
super(fallBackProvider, config.getMinStableGasPrice(), config.getRefreshRate());
super(fallBackProvider, config.getMinStableGasPrice(), config.getRefreshRate(), config.getMinValidPrice(), config.getMaxValidPrice());
this.ethModuleSupplier = ethModuleSupplier;
EthCallMinGasPriceSystemConfig ethCallConfig = config.getEthCallConfig();
this.toAddress = ethCallConfig.getAddress();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public HttpGetMinGasPriceProvider(StableMinGasPriceSystemConfig config, MinGasPr
}

public HttpGetMinGasPriceProvider(StableMinGasPriceSystemConfig config, MinGasPriceProvider fallBackProvider, SimpleHttpClient httpClient) {
super(fallBackProvider, config.getMinStableGasPrice(), config.getRefreshRate());
super(fallBackProvider, config.getMinStableGasPrice(), config.getRefreshRate(), config.getMinValidPrice(), config.getMaxValidPrice());
HttpGetStableMinGasSystemConfig httpGetConfig = config.getHttpGetConfig();
this.url = httpGetConfig.getUrl();
this.jsonPath = JsonPointer.valueOf(httpGetConfig.getJsonPath());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,23 @@
public abstract class StableMinGasPriceProvider implements MinGasPriceProvider {
private static final Logger logger = LoggerFactory.getLogger("StableMinGasPrice");
private static final int ERR_NUM_OF_FAILURES = 20;

private final MinGasPriceProvider fallBackProvider;
private final long minStableGasPrice;
private final long refreshRateInMillis;
private final long minValidPrice;
private final long maxValidPrice;
private final AtomicInteger numOfFailures = new AtomicInteger();
private final AtomicReference<Future<Long>> priceFuture = new AtomicReference<>();

private volatile long lastMinGasPrice;
private volatile long lastUpdateTimeMillis;

protected StableMinGasPriceProvider(MinGasPriceProvider fallBackProvider, long minStableGasPrice, Duration refreshRate) {
protected StableMinGasPriceProvider(MinGasPriceProvider fallBackProvider, long minStableGasPrice, Duration refreshRate, long minValidPrice, long maxValidPrice) {
this.minStableGasPrice = minStableGasPrice;
this.fallBackProvider = fallBackProvider;
this.refreshRateInMillis = refreshRate.toMillis();
this.minValidPrice = minValidPrice;
this.maxValidPrice = maxValidPrice;
}

protected abstract Optional<Long> getBtcExchangeRate();
Expand Down Expand Up @@ -128,7 +131,7 @@ private synchronized Future<Long> fetchPriceAsync() {
private Optional<Long> fetchPriceSync() {
logger.debug("fetchPriceSync...");
Optional<Long> priceResponse = getBtcExchangeRate();
if (priceResponse.isPresent() && priceResponse.get() > 0) {
if (priceResponse.isPresent() && isIntoRange(priceResponse.get())) {
long result = calculateMinGasPriceBasedOnBtcPrice(priceResponse.get());
lastMinGasPrice = result;
lastUpdateTimeMillis = System.currentTimeMillis();
Expand All @@ -148,4 +151,13 @@ private Optional<Long> fetchPriceSync() {

return Optional.empty();
}

@VisibleForTesting
boolean isIntoRange(long value) {
fmacleal marked this conversation as resolved.
Show resolved Hide resolved
if (value >= minValidPrice && value <= maxValidPrice) {
return true;
}
logger.warn("Gas price value {} is not in the valid range {} - {}", value, minValidPrice, maxValidPrice);
return false;
}
}
2 changes: 2 additions & 0 deletions rskj-core/src/main/resources/expected.conf
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ miner = {
}
stableGasPrice {
enabled = <enabled>
minValidPrice = <number>
maxValidPrice = <number>
source {
method = <method>
params {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,22 @@
import co.rsk.core.Coin;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import java.time.Duration;
import java.util.Optional;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.mockito.Mockito.*;

class StableMinGasPriceProviderTest {
private static final Long FALLBACK_PRICE = 10L;
private MinGasPriceProvider fallBackProvider;
private StableMinGasPriceProvider stableMinGasPriceProvider;

@BeforeEach
void setUp() {
fallBackProvider = Mockito.mock(MinGasPriceProvider.class);
when(fallBackProvider.getMinGasPrice()).thenReturn(10L);
when(fallBackProvider.getType()).thenReturn(MinGasPriceProviderType.FIXED);
fallBackProvider = spy(new FixedMinGasPriceProvider(FALLBACK_PRICE));
stableMinGasPriceProvider = new TestStableMingGasPriceProvider(fallBackProvider, 100, Duration.ofSeconds(10));
}

Expand Down Expand Up @@ -64,9 +63,6 @@ public Optional<Long> getBtcExchangeRate() {

@Test
void whenRequestingTheValueTwiceCachedValueIsUsed() {
MinGasPriceProvider fallbackProvider = mock(MinGasPriceProvider.class);
when(fallbackProvider.getType()).thenReturn(MinGasPriceProviderType.FIXED);

stableMinGasPriceProvider = spy(new TestStableMingGasPriceProvider(fallBackProvider, 100, Duration.ofSeconds(10)));

stableMinGasPriceProvider.getMinGasPrice();
Expand All @@ -81,11 +77,35 @@ void testGetMinGasPriceAsCoin() {
assertEquals(Coin.valueOf(6L), result);
}

@Test
void fallBackPriceIsReturnedWhenProvidedValueIsAboveMaxRange() {
StableMinGasPriceProvider spyProvider = spy(stableMinGasPriceProvider);
doReturn(Optional.of(500L)).when(spyProvider).getBtcExchangeRate();

Long price = spyProvider.getMinGasPrice(true);

boolean isIntoRangeResult = verify(spyProvider, times(1)).isIntoRange(500L);
assertFalse(isIntoRangeResult);
assertEquals(FALLBACK_PRICE, price);
}

@Test
void fallBackPriceIsReturnedWhenProvidedValueIsBelowMinRange() {
StableMinGasPriceProvider spyProvider = spy(stableMinGasPriceProvider);
doReturn(Optional.of(5L)).when(spyProvider).getBtcExchangeRate();

Long price = spyProvider.getMinGasPrice(true);

boolean isIntoRangeResult = verify(spyProvider, times(1)).isIntoRange(5L);
assertFalse(isIntoRangeResult);
assertEquals(FALLBACK_PRICE, price);
}


public static class TestStableMingGasPriceProvider extends StableMinGasPriceProvider {

protected TestStableMingGasPriceProvider(MinGasPriceProvider fallBackProvider, long minStableGasPrice, Duration refreshRate) {
super(fallBackProvider, minStableGasPrice, refreshRate);
super(fallBackProvider, minStableGasPrice, refreshRate,10L, 100L);
}

@Override
Expand Down
Loading