Merge pull request #6700 from HenrikJannsen/fix-bm-receive-address-selection

Fix BM receive address selection

core/src/main/java/bisq/core/dao/burningman/BtcFeeReceiverService.java

@@ -102,7 +102,9 @@ public class BtcFeeReceiverService implements DaoStateListener {
             // 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));
     }
 

core/src/main/java/bisq/core/dao/burningman/BurningManService.java

@@ -135,26 +135,33 @@ public class BurningManService {
                                 BurningManCandidate candidate = burningManCandidatesByName.get(name);
 
                                 // Issuance
-                                compensationProposal.getBurningManReceiverAddress()
+                                Optional<String> customAddress = compensationProposal.getBurningManReceiverAddress();
-                                        .or(() -> daoStateService.getTx(compensationProposal.getTxId())
+                                boolean isCustomAddress = customAddress.isPresent();
-                                                .map(this::getAddressFromCompensationRequest))
+                                Optional<String> receiverAddress;
-                                        .ifPresent(address -> {
+                                if (isCustomAddress) {
-                                            int issuanceHeight = issuance.getChainHeight();
+                                    receiverAddress = customAddress;
-                                            long issuanceAmount = getIssuanceAmountForCompensationRequest(issuance);
+                                } else {
-                                            int cycleIndex = cyclesInDaoStateService.getCycleIndexAtChainHeight(issuanceHeight);
+                                    // We take change address from compensation request
-                                            if (isValidCompensationRequest(name, cycleIndex, issuanceAmount)) {
+                                    receiverAddress = daoStateService.getTx(compensationProposal.getTxId())
-                                                long decayedIssuanceAmount = getDecayedCompensationAmount(issuanceAmount, issuanceHeight, chainHeight);
+                                            .map(this::getAddressFromCompensationRequest);
-                                                long issuanceDate = daoStateService.getBlockTime(issuanceHeight);
+                                }
-                                                candidate.addCompensationModel(CompensationModel.fromCompensationRequest(address,
+                                if (receiverAddress.isPresent()) {
-                                                        issuanceAmount,
+                                    int issuanceHeight = issuance.getChainHeight();
-                                                        decayedIssuanceAmount,
+                                    long issuanceAmount = getIssuanceAmountForCompensationRequest(issuance);
-                                                        issuanceHeight,
+                                    int cycleIndex = cyclesInDaoStateService.getCycleIndexAtChainHeight(issuanceHeight);
-                                                        issuance.getTxId(),
+                                    if (isValidCompensationRequest(name, cycleIndex, issuanceAmount)) {
-                                                        issuanceDate,
+                                        long decayedIssuanceAmount = getDecayedCompensationAmount(issuanceAmount, issuanceHeight, chainHeight);
-                                                        cycleIndex));
+                                        long issuanceDate = daoStateService.getBlockTime(issuanceHeight);
-                                            }
+                                        candidate.addCompensationModel(CompensationModel.fromCompensationRequest(receiverAddress.get(),
-                                        });
+                                                isCustomAddress,
-
+                                                issuanceAmount,
+                                                decayedIssuanceAmount,
+                                                issuanceHeight,
+                                                issuance.getTxId(),
+                                                issuanceDate,
+                                                cycleIndex));
+                                    }
+                                }
                                 addBurnOutputModel(chainHeight, proofOfBurnOpReturnTxOutputByHash, name, candidate);
                             });
                         }
@@ -211,7 +218,7 @@ public class BurningManService {
     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());
     }
 

core/src/main/java/bisq/core/dao/burningman/DelayedPayoutTxReceiverService.java

@@ -52,10 +52,13 @@ import static com.google.common.base.Preconditions.checkArgument;
 @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() {
+    public static boolean isBugfix6699Activated() {
-        return new Date().after(HOTFIX_ACTIVATION_DATE);
+        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).
@@ -121,17 +124,15 @@ public class DelayedPayoutTxReceiverService implements DaoStateListener {
     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 ?
+        Collection<BurningManCandidate> burningManCandidates = burningManService.getActiveBurningManCandidates(burningManSelectionHeight);
-                burningManService.getActiveBurningManCandidates(burningManSelectionHeight) :
-                burningManService.getBurningManCandidatesByName(burningManSelectionHeight).values();
 
         // 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.
@@ -158,13 +159,11 @@ public class DelayedPayoutTxReceiverService implements DaoStateListener {
         // 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 ?
+        long maxOutputAmount = Math.round(spendableAmount * (BurningManService.MAX_BURN_SHARE * 1.2));
-                Math.round(spendableAmount * (BurningManService.MAX_BURN_SHARE * 1.2)) :
-                Math.round(inputAmount * (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);
@@ -173,11 +172,11 @@ public class DelayedPayoutTxReceiverService implements DaoStateListener {
                 .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)
@@ -189,8 +188,7 @@ public class DelayedPayoutTxReceiverService implements DaoStateListener {
             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 > DPT_MIN_REMAINDER_TO_LEGACY_BM) {
-            if (available > dptMinRemainderToLegacyBm) {
                 receivers.add(new Tuple2<>(available, burningManService.getLegacyBurningManAddress(burningManSelectionHeight)));
             }
         }

core/src/main/java/bisq/core/dao/burningman/model/BurningManCandidate.java

@@ -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;
 
@@ -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<>();
@@ -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;
@@ -87,6 +108,25 @@ public class BurningManCandidate {
         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);
@@ -145,6 +185,7 @@ public class BurningManCandidate {
                 ",\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 +

core/src/main/java/bisq/core/dao/burningman/model/CompensationModel.java

@@ -27,6 +27,7 @@ import lombok.Getter;
 @EqualsAndHashCode
 public final class CompensationModel {
     public static CompensationModel fromCompensationRequest(String address,
+                                                            boolean isCustomAddress,
                                                             long amount,
                                                             long decayedAmount,
                                                             int height,
@@ -34,6 +35,7 @@ public final class CompensationModel {
                                                             long date,
                                                             int cycleIndex) {
         return new CompensationModel(address,
+                isCustomAddress,
                 amount,
                 decayedAmount,
                 height,
@@ -51,6 +53,7 @@ public final class CompensationModel {
                                                       int outputIndex,
                                                       long date) {
         return new CompensationModel(address,
+                false,
                 amount,
                 decayedAmount,
                 height,
@@ -62,6 +65,7 @@ public final class CompensationModel {
 
 
     private final String address;
+    private final boolean isCustomAddress;
     private final long amount;
     private final long decayedAmount;
     private final int height;
@@ -71,6 +75,7 @@ public final class CompensationModel {
     private final int cycleIndex;
 
     private CompensationModel(String address,
+                              boolean isCustomAddress,
                               long amount,
                               long decayedAmount,
                               int height,
@@ -79,6 +84,7 @@ public final class CompensationModel {
                               long date,
                               int cycleIndex) {
         this.address = address;
+        this.isCustomAddress = isCustomAddress;
         this.amount = amount;
         this.decayedAmount = decayedAmount;
         this.height = height;
@@ -92,6 +98,7 @@ public final class CompensationModel {
     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 +

core/src/main/java/bisq/core/dao/burningman/model/LegacyBurningMan.java

@@ -30,7 +30,7 @@ import lombok.extern.slf4j.Slf4j;
 @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) {
@@ -56,6 +56,6 @@ public final class LegacyBurningMan extends BurningManCandidate {
 
     @Override
     public Set<String> getAllAddresses() {
-        return mostRecentAddress.map(Set::of).orElse(new HashSet<>());
+        return getReceiverAddress().map(Set::of).orElse(new HashSet<>());
     }
 }

core/src/main/java/bisq/core/support/dispute/refund/RefundManager.java

@@ -329,12 +329,12 @@ public final class RefundManager extends DisputeManager<RefundDisputeList> {
         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");

desktop/src/main/java/bisq/desktop/main/dao/burnbsq/burningman/BurningManListItem.java

@@ -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();