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

Fix BM receive address selection #6700

Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ public String getAddress() {
// the burningManCandidates as we added for the legacy BM an entry at the end.
return burningManService.getLegacyBurningManAddress(currentChainHeight);
}
return activeBurningManCandidates.get(winnerIndex).getMostRecentAddress()
// For the fee selection we do not need to wait for activation date of the bugfix for
// the receiver address (https://github.com/bisq-network/bisq/issues/6699) as it has no impact on the trade protocol.
return activeBurningManCandidates.get(winnerIndex).getReceiverAddress(true)
.orElse(burningManService.getLegacyBurningManAddress(currentChainHeight));
}

Expand Down
49 changes: 28 additions & 21 deletions core/src/main/java/bisq/core/dao/burningman/BurningManService.java
Original file line number Diff line number Diff line change
Expand Up @@ -135,26 +135,33 @@ Map<String, BurningManCandidate> getBurningManCandidatesByName(int chainHeight)
BurningManCandidate candidate = burningManCandidatesByName.get(name);

// Issuance
compensationProposal.getBurningManReceiverAddress()
.or(() -> daoStateService.getTx(compensationProposal.getTxId())
.map(this::getAddressFromCompensationRequest))
.ifPresent(address -> {
int issuanceHeight = issuance.getChainHeight();
long issuanceAmount = getIssuanceAmountForCompensationRequest(issuance);
int cycleIndex = cyclesInDaoStateService.getCycleIndexAtChainHeight(issuanceHeight);
if (isValidCompensationRequest(name, cycleIndex, issuanceAmount)) {
long decayedIssuanceAmount = getDecayedCompensationAmount(issuanceAmount, issuanceHeight, chainHeight);
long issuanceDate = daoStateService.getBlockTime(issuanceHeight);
candidate.addCompensationModel(CompensationModel.fromCompensationRequest(address,
issuanceAmount,
decayedIssuanceAmount,
issuanceHeight,
issuance.getTxId(),
issuanceDate,
cycleIndex));
}
});

Optional<String> customAddress = compensationProposal.getBurningManReceiverAddress();
boolean isCustomAddress = customAddress.isPresent();
Optional<String> receiverAddress;
if (isCustomAddress) {
receiverAddress = customAddress;
} else {
// We take change address from compensation request
receiverAddress = daoStateService.getTx(compensationProposal.getTxId())
.map(this::getAddressFromCompensationRequest);
}
if (receiverAddress.isPresent()) {
int issuanceHeight = issuance.getChainHeight();
long issuanceAmount = getIssuanceAmountForCompensationRequest(issuance);
int cycleIndex = cyclesInDaoStateService.getCycleIndexAtChainHeight(issuanceHeight);
if (isValidCompensationRequest(name, cycleIndex, issuanceAmount)) {
long decayedIssuanceAmount = getDecayedCompensationAmount(issuanceAmount, issuanceHeight, chainHeight);
long issuanceDate = daoStateService.getBlockTime(issuanceHeight);
candidate.addCompensationModel(CompensationModel.fromCompensationRequest(receiverAddress.get(),
isCustomAddress,
issuanceAmount,
decayedIssuanceAmount,
issuanceHeight,
issuance.getTxId(),
issuanceDate,
cycleIndex));
}
}
addBurnOutputModel(chainHeight, proofOfBurnOpReturnTxOutputByHash, name, candidate);
});
}
Expand Down Expand Up @@ -211,7 +218,7 @@ String getLegacyBurningManAddress(int chainHeight) {
Set<BurningManCandidate> getActiveBurningManCandidates(int chainHeight) {
return getBurningManCandidatesByName(chainHeight).values().stream()
.filter(burningManCandidate -> burningManCandidate.getCappedBurnAmountShare() > 0)
.filter(candidate -> candidate.getMostRecentAddress().isPresent())
.filter(candidate -> candidate.getReceiverAddress().isPresent())
.collect(Collectors.toSet());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,13 @@
@Slf4j
@Singleton
public class DelayedPayoutTxReceiverService implements DaoStateListener {
public static final Date HOTFIX_ACTIVATION_DATE = Utilities.getUTCDate(2023, GregorianCalendar.JANUARY, 10);
// Activation date for bugfix of receiver addresses getting overwritten by a new compensation
// requests change address.
// See: https://github.com/bisq-network/bisq/issues/6699
public static final Date BUGFIX_6699_ACTIVATION_DATE = Utilities.getUTCDate(2023, GregorianCalendar.JULY, 15);

public static boolean isHotfixActivated() {
return new Date().after(HOTFIX_ACTIVATION_DATE);
public static boolean isBugfix6699Activated() {
return new Date().after(BUGFIX_6699_ACTIVATION_DATE);
}

// We don't allow to get further back than 767950 (the block height from Dec. 18th 2022).
Expand Down Expand Up @@ -121,17 +124,15 @@ public int getBurningManSelectionHeight() {
public List<Tuple2<Long, String>> getReceivers(int burningManSelectionHeight,
long inputAmount,
long tradeTxFee) {
return getReceivers(burningManSelectionHeight, inputAmount, tradeTxFee, isHotfixActivated());
return getReceivers(burningManSelectionHeight, inputAmount, tradeTxFee, isBugfix6699Activated());
}

public List<Tuple2<Long, String>> getReceivers(int burningManSelectionHeight,
long inputAmount,
long tradeTxFee,
boolean isHotfixActivated) {
boolean isBugfix6699Activated) {
checkArgument(burningManSelectionHeight >= MIN_SNAPSHOT_HEIGHT, "Selection height must be >= " + MIN_SNAPSHOT_HEIGHT);
Collection<BurningManCandidate> burningManCandidates = isHotfixActivated ?
burningManService.getActiveBurningManCandidates(burningManSelectionHeight) :
burningManService.getBurningManCandidatesByName(burningManSelectionHeight).values();
Collection<BurningManCandidate> burningManCandidates = burningManService.getActiveBurningManCandidates(burningManSelectionHeight);

// We need to use the same txFeePerVbyte value for both traders.
// We use the tradeTxFee value which is calculated from the average of taker fee tx size and deposit tx size.
Expand All @@ -158,13 +159,11 @@ public List<Tuple2<Long, String>> getReceivers(int burningManSelectionHeight,
// If we remove outputs it will be spent as miner fee.
long minOutputAmount = Math.max(DPT_MIN_OUTPUT_AMOUNT, txFeePerVbyte * 32 * 2);
// Sanity check that max share of a non-legacy BM is 20% over MAX_BURN_SHARE (taking into account potential increase due adjustment)
long maxOutputAmount = isHotfixActivated ?
Math.round(spendableAmount * (BurningManService.MAX_BURN_SHARE * 1.2)) :
Math.round(inputAmount * (BurningManService.MAX_BURN_SHARE * 1.2));
long maxOutputAmount = Math.round(spendableAmount * (BurningManService.MAX_BURN_SHARE * 1.2));
// We accumulate small amounts which gets filtered out and subtract it from 1 to get an adjustment factor
// used later to be applied to the remaining burningmen share.
double adjustment = 1 - burningManCandidates.stream()
.filter(candidate -> candidate.getMostRecentAddress().isPresent())
.filter(candidate -> candidate.getReceiverAddress(isBugfix6699Activated).isPresent())
.mapToDouble(candidate -> {
double cappedBurnAmountShare = candidate.getCappedBurnAmountShare();
long amount = Math.round(cappedBurnAmountShare * spendableAmount);
Expand All @@ -173,11 +172,11 @@ public List<Tuple2<Long, String>> getReceivers(int burningManSelectionHeight,
.sum();

List<Tuple2<Long, String>> receivers = burningManCandidates.stream()
.filter(candidate -> candidate.getMostRecentAddress().isPresent())
.filter(candidate -> candidate.getReceiverAddress(isBugfix6699Activated).isPresent())
.map(candidate -> {
double cappedBurnAmountShare = candidate.getCappedBurnAmountShare() / adjustment;
return new Tuple2<>(Math.round(cappedBurnAmountShare * spendableAmount),
candidate.getMostRecentAddress().get());
candidate.getReceiverAddress(isBugfix6699Activated).get());
})
.filter(tuple -> tuple.first >= minOutputAmount)
.filter(tuple -> tuple.first <= maxOutputAmount)
Expand All @@ -189,8 +188,7 @@ public List<Tuple2<Long, String>> getReceivers(int burningManSelectionHeight,
long available = spendableAmount - totalOutputValue;
// If the available is larger than DPT_MIN_REMAINDER_TO_LEGACY_BM we send it to legacy BM
// Otherwise we use it as miner fee
long dptMinRemainderToLegacyBm = isHotfixActivated ? DPT_MIN_REMAINDER_TO_LEGACY_BM : 50000;
if (available > dptMinRemainderToLegacyBm) {
if (available > DPT_MIN_REMAINDER_TO_LEGACY_BM) {
receivers.add(new Tuple2<>(available, burningManService.getLegacyBurningManAddress(burningManSelectionHeight)));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package bisq.core.dao.burningman.model;

import bisq.core.dao.burningman.BurningManService;
import bisq.core.dao.burningman.DelayedPayoutTxReceiverService;

import bisq.common.util.DateUtil;

Expand Down Expand Up @@ -46,6 +47,13 @@ public class BurningManCandidate {
private long accumulatedCompensationAmount;
private long accumulatedDecayedCompensationAmount;
private double compensationShare; // Share of accumulated decayed compensation amounts in relation to total issued amounts

protected Optional<String> receiverAddress = Optional.empty();

// For deploying a bugfix with mostRecentAddress we need to maintain the old version to avoid breaking the
// trade protocol. We use the legacyMostRecentAddress until the activation date where we
// enforce the version by the filter to ensure users have updated.
// See: https://github.com/bisq-network/bisq/issues/6699
protected Optional<String> mostRecentAddress = Optional.empty();

private final Set<BurnOutputModel> burnOutputModels = new HashSet<>();
Expand All @@ -63,6 +71,19 @@ public class BurningManCandidate {
public BurningManCandidate() {
}

public Optional<String> getReceiverAddress() {
return getReceiverAddress(DelayedPayoutTxReceiverService.isBugfix6699Activated());
}

public Optional<String> getReceiverAddress(boolean isBugfix6699Activated) {
return isBugfix6699Activated ? receiverAddress : mostRecentAddress;
}

public Optional<String> getMostRecentAddress() {
// Lombok getter is set for class, so we would get a getMostRecentAddress but want to ensure it's not accidentally used.
throw new UnsupportedOperationException("getMostRecentAddress must not be used. Use getReceiverAddress instead.");
}

public void addBurnOutputModel(BurnOutputModel burnOutputModel) {
if (burnOutputModels.contains(burnOutputModel)) {
return;
Expand All @@ -87,6 +108,25 @@ public void addCompensationModel(CompensationModel compensationModel) {
accumulatedDecayedCompensationAmount += compensationModel.getDecayedAmount();
accumulatedCompensationAmount += compensationModel.getAmount();

boolean hasAnyCustomAddress = compensationModels.stream()
.anyMatch(CompensationModel::isCustomAddress);
if (hasAnyCustomAddress) {
// If any custom address was defined, we only consider custom addresses and sort them to take the
// most recent one.
receiverAddress = compensationModels.stream()
.filter(CompensationModel::isCustomAddress)
.max(Comparator.comparing(CompensationModel::getHeight))
.map(CompensationModel::getAddress);
} else {
// If no custom addresses ever have been defined, we take the change address of the compensation request
// and use the earliest address. This helps to avoid change of address with every new comp. request.
receiverAddress = compensationModels.stream()
.min(Comparator.comparing(CompensationModel::getHeight))
.map(CompensationModel::getAddress);
}

// For backward compatibility reasons we need to maintain the old buggy version.
// See: https://github.com/bisq-network/bisq/issues/6699.
mostRecentAddress = compensationModels.stream()
.max(Comparator.comparing(CompensationModel::getHeight))
.map(CompensationModel::getAddress);
Expand Down Expand Up @@ -145,6 +185,7 @@ public String toString() {
",\r\n accumulatedCompensationAmount=" + accumulatedCompensationAmount +
",\r\n accumulatedDecayedCompensationAmount=" + accumulatedDecayedCompensationAmount +
",\r\n compensationShare=" + compensationShare +
",\r\n receiverAddress=" + receiverAddress +
",\r\n mostRecentAddress=" + mostRecentAddress +
",\r\n burnOutputModels=" + burnOutputModels +
",\r\n accumulatedBurnAmount=" + accumulatedBurnAmount +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@
@EqualsAndHashCode
public final class CompensationModel {
public static CompensationModel fromCompensationRequest(String address,
boolean isCustomAddress,
long amount,
long decayedAmount,
int height,
String txId,
long date,
int cycleIndex) {
return new CompensationModel(address,
isCustomAddress,
amount,
decayedAmount,
height,
Expand All @@ -51,6 +53,7 @@ public static CompensationModel fromGenesisOutput(String address,
int outputIndex,
long date) {
return new CompensationModel(address,
false,
amount,
decayedAmount,
height,
Expand All @@ -62,6 +65,7 @@ public static CompensationModel fromGenesisOutput(String address,


private final String address;
private final boolean isCustomAddress;
private final long amount;
private final long decayedAmount;
private final int height;
Expand All @@ -71,6 +75,7 @@ public static CompensationModel fromGenesisOutput(String address,
private final int cycleIndex;

private CompensationModel(String address,
boolean isCustomAddress,
long amount,
long decayedAmount,
int height,
Expand All @@ -79,6 +84,7 @@ private CompensationModel(String address,
long date,
int cycleIndex) {
this.address = address;
this.isCustomAddress = isCustomAddress;
this.amount = amount;
this.decayedAmount = decayedAmount;
this.height = height;
Expand All @@ -92,6 +98,7 @@ private CompensationModel(String address,
public String toString() {
return "\n CompensationModel{" +
"\r\n address='" + address + '\'' +
"\r\n isCustomAddress='" + isCustomAddress + '\'' +
",\r\n amount=" + amount +
",\r\n decayedAmount=" + decayedAmount +
",\r\n height=" + height +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
@EqualsAndHashCode(callSuper = true)
public final class LegacyBurningMan extends BurningManCandidate {
public LegacyBurningMan(String address) {
mostRecentAddress = Optional.of(address);
receiverAddress = mostRecentAddress = Optional.of(address);
}

public void applyBurnAmountShare(double burnAmountShare) {
Expand All @@ -56,6 +56,6 @@ public void calculateCappedAndAdjustedShares(double sumAllCappedBurnAmountShares

@Override
public Set<String> getAllAddresses() {
return mostRecentAddress.map(Set::of).orElse(new HashSet<>());
return getReceiverAddress().map(Set::of).orElse(new HashSet<>());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -329,12 +329,12 @@ public void verifyDelayedPayoutTxReceivers(Transaction delayedPayoutTx, Dispute
long inputAmount = depositTx.getOutput(0).getValue().value;
int selectionHeight = dispute.getBurningManSelectionHeight();

boolean wasHotfixActivatedAtTradeDate = dispute.getTradeDate().after(DelayedPayoutTxReceiverService.HOTFIX_ACTIVATION_DATE);
boolean wasBugfix6699ActivatedAtTradeDate = dispute.getTradeDate().after(DelayedPayoutTxReceiverService.BUGFIX_6699_ACTIVATION_DATE);
List<Tuple2<Long, String>> delayedPayoutTxReceivers = delayedPayoutTxReceiverService.getReceivers(
selectionHeight,
inputAmount,
dispute.getTradeTxFee(),
wasHotfixActivatedAtTradeDate);
wasBugfix6699ActivatedAtTradeDate);
log.info("Verify delayedPayoutTx using selectionHeight {} and receivers {}", selectionHeight, delayedPayoutTxReceivers);
checkArgument(delayedPayoutTx.getOutputs().size() == delayedPayoutTxReceivers.size(),
"Size of outputs and delayedPayoutTxReceivers must be the same");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class BurningManListItem {
this.burningManCandidate = burningManCandidate;

this.name = name;
address = burningManCandidate.getMostRecentAddress().orElse(Res.get("shared.na"));
address = burningManCandidate.getReceiverAddress().orElse(Res.get("shared.na"));

adjustedBurnAmountShare = burningManCandidate.getAdjustedBurnAmountShare();
cappedBurnAmountShare = burningManCandidate.getCappedBurnAmountShare();
Expand Down