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

Obtain minimumFee from mempool api in place of hardcoded value #5235

Merged
merged 3 commits into from Mar 1, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions common/src/main/java/bisq/common/config/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ public class Config {
public static final String API_PORT = "apiPort";
public static final String PREVENT_PERIODIC_SHUTDOWN_AT_SEED_NODE = "preventPeriodicShutdownAtSeedNode";
public static final String REPUBLISH_MAILBOX_ENTRIES = "republishMailboxEntries";
public static final String BTC_TX_FEE = "btcTxFee";
public static final String BTC_MIN_TX_FEE = "btcMinTxFee";
public static final String BTC_FEES_TS = "bitcoinFeesTs";

// Default values for certain options
public static final int UNSPECIFIED_PORT = -1;
Expand Down
10 changes: 7 additions & 3 deletions core/src/main/java/bisq/core/provider/fee/FeeProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import bisq.network.http.HttpClient;

import bisq.common.app.Version;
import bisq.common.config.Config;
import bisq.common.util.Tuple2;

import com.google.gson.Gson;
Expand Down Expand Up @@ -51,15 +52,18 @@ public Tuple2<Map<String, Long>, Map<String, Long>> getFees() throws IOException

LinkedTreeMap<?, ?> linkedTreeMap = new Gson().fromJson(json, LinkedTreeMap.class);
Map<String, Long> tsMap = new HashMap<>();
tsMap.put("bitcoinFeesTs", ((Double) linkedTreeMap.get("bitcoinFeesTs")).longValue());
tsMap.put(Config.BTC_FEES_TS, ((Double) linkedTreeMap.get(Config.BTC_FEES_TS)).longValue());

Map<String, Long> map = new HashMap<>();

try {
LinkedTreeMap<?, ?> dataMap = (LinkedTreeMap<?, ?>) linkedTreeMap.get("dataMap");
Long btcTxFee = ((Double) dataMap.get("btcTxFee")).longValue();
Long btcTxFee = ((Double) dataMap.get(Config.BTC_TX_FEE)).longValue();
Long btcMinTxFee = dataMap.get(Config.BTC_MIN_TX_FEE) != null ?
((Double) dataMap.get(Config.BTC_MIN_TX_FEE)).longValue() : Config.baseCurrencyNetwork().getDefaultMinFeePerVbyte();

map.put("BTC", btcTxFee);
map.put(Config.BTC_TX_FEE, btcTxFee);
map.put(Config.BTC_MIN_TX_FEE, btcMinTxFee);
} catch (Throwable t) {
log.error(t.toString());
t.printStackTrace();
Expand Down
10 changes: 6 additions & 4 deletions core/src/main/java/bisq/core/provider/fee/FeeService.java
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public static Coin getMinTakerFee(boolean currencyForFeeIsBtc) {
private Map<String, Long> timeStampMap;
@Getter
private long lastRequest;
@Getter
private long minFeePerVByte;
private long epochInSecondAtLastRequest;

Expand Down Expand Up @@ -154,17 +155,18 @@ public void onSuccess(@Nullable Tuple2<Map<String, Long>, Map<String, Long>> res
UserThread.execute(() -> {
checkNotNull(result, "Result must not be null at getFees");
timeStampMap = result.first;
epochInSecondAtLastRequest = timeStampMap.get("bitcoinFeesTs");
epochInSecondAtLastRequest = timeStampMap.get(Config.BTC_FEES_TS);
final Map<String, Long> map = result.second;
txFeePerVbyte = map.get("BTC");
txFeePerVbyte = map.get(Config.BTC_TX_FEE);
minFeePerVByte = map.get(Config.BTC_MIN_TX_FEE);

if (txFeePerVbyte < minFeePerVByte) {
log.warn("The delivered fee per vbyte is smaller than the min. default fee of 5 sat/vbyte");
log.warn("The delivered fee of {} sat/vbyte is smaller than the min. default fee of {} sat/vbyte", txFeePerVbyte, minFeePerVByte);
txFeePerVbyte = minFeePerVByte;
}

feeUpdateCounter.set(feeUpdateCounter.get() + 1);
log.info("BTC tx fee: txFeePerVbyte={}", txFeePerVbyte);
log.info("BTC tx fee: txFeePerVbyte={} minFeePerVbyte={}", txFeePerVbyte, minFeePerVByte);
if (resultHandler != null)
resultHandler.run();
});
Expand Down
6 changes: 5 additions & 1 deletion core/src/main/java/bisq/core/user/Preferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import bisq.core.locale.TradeCurrency;
import bisq.core.payment.PaymentAccount;
import bisq.core.payment.PaymentAccountUtil;
import bisq.core.provider.fee.FeeService;
import bisq.core.setup.CoreNetworkCapabilities;

import bisq.network.p2p.network.BridgeAddressProvider;
Expand Down Expand Up @@ -165,6 +166,7 @@ public final class Preferences implements PersistedDataHost, BridgeAddressProvid

private final PersistenceManager<PreferencesPayload> persistenceManager;
private final Config config;
private final FeeService feeService;
private final LocalBitcoinNode localBitcoinNode;
private final String btcNodesFromOptions, referralIdFromOptions,
rpcUserFromOptions, rpcPwFromOptions;
Expand All @@ -181,6 +183,7 @@ public final class Preferences implements PersistedDataHost, BridgeAddressProvid
@Inject
public Preferences(PersistenceManager<PreferencesPayload> persistenceManager,
Config config,
FeeService feeService,
LocalBitcoinNode localBitcoinNode,
@Named(Config.BTC_NODES) String btcNodesFromOptions,
@Named(Config.REFERRAL_ID) String referralId,
Expand All @@ -191,6 +194,7 @@ public Preferences(PersistenceManager<PreferencesPayload> persistenceManager,

this.persistenceManager = persistenceManager;
this.config = config;
this.feeService = feeService;
this.localBitcoinNode = localBitcoinNode;
this.btcNodesFromOptions = btcNodesFromOptions;
this.referralIdFromOptions = referralId;
Expand Down Expand Up @@ -907,7 +911,7 @@ public List<String> getBridgeAddresses() {

public long getWithdrawalTxFeeInVbytes() {
return Math.max(prefPayload.getWithdrawalTxFeeInVbytes(),
Config.baseCurrencyNetwork().getDefaultMinFeePerVbyte());
This conversation was marked as resolved.
Show resolved Hide resolved
feeService.getMinFeePerVByte());
}

public boolean isDaoFullNode() {
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/bisq/core/user/PreferencesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void setUp() {
Config config = new Config();
LocalBitcoinNode localBitcoinNode = new LocalBitcoinNode(config);
preferences = new Preferences(
persistenceManager, config, localBitcoinNode, null, null, Config.DEFAULT_FULL_DAO_NODE,
persistenceManager, config, null, localBitcoinNode, null, null, Config.DEFAULT_FULL_DAO_NODE,
null, null, Config.UNSPECIFIED_PORT);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ private void initializeGeneralOptions() {
String estimatedFee = String.valueOf(feeService.getTxFeePerVbyte().value);
try {
int withdrawalTxFeePerVbyte = Integer.parseInt(transactionFeeInputTextField.getText());
final long minFeePerVbyte = Config.baseCurrencyNetwork().getDefaultMinFeePerVbyte();
final long minFeePerVbyte = feeService.getMinFeePerVByte();
if (withdrawalTxFeePerVbyte < minFeePerVbyte) {
new Popup().warning(Res.get("setting.preferences.txFeeMin", minFeePerVbyte)).show();
transactionFeeInputTextField.setText(estimatedFee);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package bisq.desktop.maker;

import bisq.core.btc.nodes.LocalBitcoinNode;
import bisq.core.provider.fee.FeeService;
import bisq.core.user.Preferences;

import bisq.common.config.Config;
Expand All @@ -34,13 +35,15 @@ public class PreferenceMakers {

public static final Property<Preferences, PersistenceManager> storage = new Property<>();
public static final Property<Preferences, Config> config = new Property<>();
public static final Property<Preferences, FeeService> feeService = new Property<>();
public static final Property<Preferences, LocalBitcoinNode> localBitcoinNode = new Property<>();
public static final Property<Preferences, String> useTorFlagFromOptions = new Property<>();
public static final Property<Preferences, String> referralID = new Property<>();

public static final Instantiator<Preferences> Preferences = lookup -> new Preferences(
lookup.valueOf(storage, new SameValueDonor<PersistenceManager>(null)),
lookup.valueOf(config, new SameValueDonor<Config>(null)),
lookup.valueOf(feeService, new SameValueDonor<FeeService>(null)),
lookup.valueOf(localBitcoinNode, new SameValueDonor<LocalBitcoinNode>(null)),
lookup.valueOf(useTorFlagFromOptions, new SameValueDonor<String>(null)),
lookup.valueOf(referralID, new SameValueDonor<String>(null)),
Expand Down
8 changes: 7 additions & 1 deletion pricenode/src/main/java/bisq/price/mining/FeeRate.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ public class FeeRate {

private final String currency;
private final long price;
private final long minimumFee;
private final long timestamp;

public FeeRate(String currency, long price, long timestamp) {
public FeeRate(String currency, long price, long minimumFee, long timestamp) {
this.currency = currency;
this.price = price;
this.minimumFee = minimumFee;
this.timestamp = timestamp;
}

Expand All @@ -40,6 +42,10 @@ public long getPrice() {
return price;
}

public long getMinimumFee() {
return minimumFee;
}

public long getTimestamp() {
return timestamp;
}
Expand Down
14 changes: 12 additions & 2 deletions pricenode/src/main/java/bisq/price/mining/FeeRateService.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

package bisq.price.mining;

import bisq.common.config.Config;

import org.springframework.stereotype.Service;

import java.time.Instant;
Expand Down Expand Up @@ -55,6 +57,7 @@ public Map<String, Object> getFees() {
Map<String, Long> allFeeRates = new HashMap<>();

AtomicLong sumOfAllFeeRates = new AtomicLong();
AtomicLong sumOfAllMinFeeRates = new AtomicLong();
AtomicInteger amountOfFeeRates = new AtomicInteger();

// Process each provider, retrieve and store their fee rate
Expand All @@ -67,6 +70,7 @@ public Map<String, Object> getFees() {
String currency = feeRate.getCurrency();
if ("BTC".equals(currency)) {
sumOfAllFeeRates.getAndAdd(feeRate.getPrice());
sumOfAllMinFeeRates.getAndAdd(feeRate.getMinimumFee());
amountOfFeeRates.getAndAdd(1);
}
});
Expand All @@ -75,18 +79,24 @@ public Map<String, Object> getFees() {
long averageFeeRate = (amountOfFeeRates.intValue() > 0)
? sumOfAllFeeRates.longValue() / amountOfFeeRates.intValue()
: FeeRateProvider.MIN_FEE_RATE;
long averageMinFeeRate = (amountOfFeeRates.intValue() > 0)
? sumOfAllMinFeeRates.longValue() / amountOfFeeRates.intValue()
: FeeRateProvider.MIN_FEE_RATE;

// Make sure the returned value is within the min-max range
averageFeeRate = Math.max(averageFeeRate, FeeRateProvider.MIN_FEE_RATE);
averageFeeRate = Math.min(averageFeeRate, FeeRateProvider.MAX_FEE_RATE);
averageMinFeeRate = Math.max(averageMinFeeRate, FeeRateProvider.MIN_FEE_RATE);
averageMinFeeRate = Math.min(averageMinFeeRate, FeeRateProvider.MAX_FEE_RATE);

// Prepare response: Add timestamp of now
// Since this is an average, the timestamp is associated with when the moment in
// time when the avg was computed
metadata.put("bitcoinFeesTs", Instant.now().getEpochSecond());
metadata.put(Config.BTC_FEES_TS, Instant.now().getEpochSecond());

// Prepare response: Add the fee average
allFeeRates.put("btcTxFee", averageFeeRate);
allFeeRates.put(Config.BTC_TX_FEE, averageFeeRate);
allFeeRates.put(Config.BTC_MIN_TX_FEE, averageMinFeeRate);

// Build response
return new HashMap<>() {{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@

import java.util.Map;
import java.util.Optional;
import java.util.stream.Stream;
import java.util.Set;

/**
* {@link FeeRateProvider} that interprets the Mempool API format to retrieve a mining
Expand Down Expand Up @@ -77,33 +77,37 @@ public MempoolFeeRateProvider(Environment env) {
protected FeeRate doGet() {
// Default value is the minimum rate. If the connection to the fee estimate
// provider fails, we fall back to this value.
long estimatedFeeRate = MIN_FEE_RATE;
try {
estimatedFeeRate = getEstimatedFeeRate();
return getEstimatedFeeRate();
}
catch (Exception e) {
// Something happened with the connection
log.error("Error retrieving bitcoin mining fee estimation: " + e.getMessage());
}

return new FeeRate("BTC", estimatedFeeRate, Instant.now().getEpochSecond());
return new FeeRate("BTC", MIN_FEE_RATE, MIN_FEE_RATE, Instant.now().getEpochSecond());
}

private long getEstimatedFeeRate() {
return getFeeRatePredictions()
.filter(p -> p.getKey().equalsIgnoreCase("halfHourFee"))
.map(Map.Entry::getValue)
.findFirst()
.map(r -> {
log.info("Retrieved estimated mining fee of {} sat/vbyte from {}", r, getMempoolApiHostname());
return r;
})
.map(r -> Math.max(r, MIN_FEE_RATE))
.map(r -> Math.min(r, MAX_FEE_RATE))
.orElse(MIN_FEE_RATE);
private FeeRate getEstimatedFeeRate() {
Set<Map.Entry<String, Long>> feeRatePredictions = getFeeRatePredictions();
long estimatedFeeRate = feeRatePredictions.stream()
.filter(p -> p.getKey().equalsIgnoreCase("halfHourFee"))
.map(Map.Entry::getValue)
.findFirst()
.map(r -> Math.max(r, MIN_FEE_RATE))
.map(r -> Math.min(r, MAX_FEE_RATE))
.orElse(MIN_FEE_RATE);
long minimumFee = feeRatePredictions.stream()
.filter(p -> p.getKey().equalsIgnoreCase("minimumFee"))
.map(Map.Entry::getValue)
.findFirst()
.map(r -> Math.multiplyExact(r, 2)) // multiply the minimumFee by 2 (per wiz)
.orElse(MIN_FEE_RATE);
log.info("Retrieved estimated mining fee of {} sat/vB and minimumFee of {} sat/vB from {}", estimatedFeeRate, minimumFee, getMempoolApiHostname());
return new FeeRate("BTC", estimatedFeeRate, minimumFee, Instant.now().getEpochSecond());
}

private Stream<Map.Entry<String, Long>> getFeeRatePredictions() {
private Set<Map.Entry<String, Long>> getFeeRatePredictions() {
return restTemplate.exchange(
RequestEntity
.get(UriComponentsBuilder
Expand All @@ -112,7 +116,7 @@ private Stream<Map.Entry<String, Long>> getFeeRatePredictions() {
.build().toUri())
.build(),
new ParameterizedTypeReference<Map<String, Long>>() { }
).getBody().entrySet().stream();
).getBody().entrySet();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import bisq.price.mining.providers.MempoolFeeRateProviderTest;

import bisq.common.config.Config;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -99,10 +101,10 @@ private void doSanityChecksForRetrievedData(Map<String, Object> retrievedData, l
// Check if the response has the expected format. Since the timestamp is that of
// the average (not that of the individual fee rates reported by the individual
// providers), we always expect a non-zero timestamp
assertNotEquals(0L, retrievedData.get("bitcoinFeesTs"));
assertNotEquals(0L, retrievedData.get(Config.BTC_FEES_TS));

Map<String, String> retrievedDataMap = (Map<String, String>) retrievedData.get("dataMap");
assertEquals(1, retrievedDataMap.size());
assertEquals(expectedFeeRate, retrievedDataMap.get("btcTxFee"));
assertEquals(2, retrievedDataMap.size());
assertEquals(expectedFeeRate, retrievedDataMap.get(Config.BTC_TX_FEE));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public static FeeRateProvider buildDummyReachableMempoolFeeRateProvider(long fee
MempoolFeeRateProvider dummyProvider = new MempoolFeeRateProvider.First(env) {
@Override
protected FeeRate doGet() {
return new FeeRate("BTC", feeRate, Instant.now().getEpochSecond());
return new FeeRate("BTC", feeRate, MIN_FEE_RATE, Instant.now().getEpochSecond());
}
};

Expand Down