Merge pull request #4918 from bisq-network/hotfix/v1.5.1

Hotfix/v1.5.1

build.gradle

@@ -28,7 +28,7 @@ configure(subprojects) {
 
     ext { // in alphabetical order
         bcVersion = '1.63'
-        bitcoinjVersion = '7752cb7'
+        bitcoinjVersion = 'dcf8af0'
         btcdCli4jVersion = '27b94333'
         codecVersion = '1.13'
         easybindVersion = '1.0.3'
@@ -386,7 +386,7 @@ configure(project(':desktop')) {
     apply plugin: 'witness'
     apply from: '../gradle/witness/gradle-witness.gradle'
 
-    version = '1.5.0-SNAPSHOT'
+    version = '1.5.1-SNAPSHOT'
 
     mainClassName = 'bisq.desktop.app.BisqAppMain'
 

common/src/main/java/bisq/common/app/Log.java

@@ -48,7 +48,7 @@ public static void setup(String fileName) {
         rollingPolicy.setParent(appender);
         rollingPolicy.setFileNamePattern(fileName + "_%i.log");
         rollingPolicy.setMinIndex(1);
-        rollingPolicy.setMaxIndex(10);
+        rollingPolicy.setMaxIndex(20);
         rollingPolicy.start();
 
         SizeBasedTriggeringPolicy<ILoggingEvent> triggeringPolicy = new SizeBasedTriggeringPolicy<>();

common/src/main/java/bisq/common/app/Version.java

@@ -30,7 +30,7 @@ public class Version {
     // VERSION = 0.5.0 introduces proto buffer for the P2P network and local DB and is a not backward compatible update
     // Therefore all sub versions start again with 1
     // We use semantic versioning with major, minor and patch
-    public static final String VERSION = "1.5.0";
+    public static final String VERSION = "1.5.1";
 
     /**
      * Holds a list of the tagged resource files for optimizing the getData requests.

common/src/main/java/bisq/common/setup/CommonSetup.java

@@ -93,7 +93,6 @@ public static void setupUncaughtExceptionHandler(UncaughtExceptionHandler uncaug
     private static void setupLog(Config config) {
         String logPath = Paths.get(config.appDataDir.getPath(), "bisq").toString();
         Log.setup(logPath);
-        log.info("Log files under: {}", logPath);
         Utilities.printSysInfo();
         Log.setLevel(Level.toLevel(config.logLevel));
     }

core/src/main/java/bisq/core/app/BisqSetup.java

@@ -467,7 +467,7 @@ private void checkForLockedUpFunds() {
                     .filter(e -> setOfAllTradeIds.contains(e.getOfferId()) &&
                             e.getContext() == AddressEntry.Context.MULTI_SIG)
                     .forEach(e -> {
-                        Coin balance = e.getCoinLockedInMultiSig();
+                        Coin balance = e.getCoinLockedInMultiSigAsCoin();
                         if (balance.isPositive()) {
                             String message = Res.get("popup.warning.lockedUpFunds",
                                     formatter.formatCoinWithCode(balance), e.getAddressString(), e.getOfferId());

core/src/main/java/bisq/core/app/WalletAppSetup.java

@@ -107,7 +107,7 @@ void init(@Nullable Consumer<String> chainFileLockedExceptionHandler,
               Runnable downloadCompleteHandler,
               Runnable walletInitializedHandler) {
         log.info("Initialize WalletAppSetup with BitcoinJ version {} and hash of BitcoinJ commit {}",
-                VersionMessage.BITCOINJ_VERSION, "7752cb7");
+                VersionMessage.BITCOINJ_VERSION, "dcf8af0");
 
         ObjectProperty<Throwable> walletServiceException = new SimpleObjectProperty<>();
         btcInfoBinding = EasyBind.combine(walletsSetup.downloadPercentageProperty(),

core/src/main/java/bisq/core/btc/Balances.java

@@ -124,7 +124,7 @@ private void updateLockedBalance() {
         long sum = lockedTrades.map(trade -> btcWalletService.getAddressEntry(trade.getId(), AddressEntry.Context.MULTI_SIG)
                 .orElse(null))
                 .filter(Objects::nonNull)
-                .mapToLong(addressEntry -> addressEntry.getCoinLockedInMultiSig().getValue())
+                .mapToLong(AddressEntry::getCoinLockedInMultiSig)
                 .sum();
         lockedBalance.set(Coin.valueOf(sum));
     }

core/src/main/java/bisq/core/btc/model/AddressEntry.java

@@ -71,16 +71,19 @@ public enum Context {
     private final byte[] pubKey;
     @Getter
     private final byte[] pubKeyHash;
-
-    private long coinLockedInMultiSig;
-
     @Getter
-    private boolean segwit;
+    private final long coinLockedInMultiSig;
+    @Getter
+    private final boolean segwit;
 
+    // Not an immutable field. Set at startup once wallet is ready and at encrypting/decrypting wallet.
     @Nullable
     transient private DeterministicKey keyPair;
+
+    // Only used as cache
     @Nullable
     transient private Address address;
+    // Only used as cache
     @Nullable
     transient private String addressString;
 
@@ -93,16 +96,29 @@ public AddressEntry(DeterministicKey keyPair, Context context, boolean segwit) {
         this(keyPair, context, null, segwit);
     }
 
-    public AddressEntry(@NotNull DeterministicKey keyPair,
+    public AddressEntry(DeterministicKey keyPair,
+                        Context context,
+                        @Nullable String offerId,
+                        boolean segwit) {
+        this(keyPair,
+                context,
+                offerId,
+                0,
+                segwit);
+    }
+
+    public AddressEntry(DeterministicKey keyPair,
                         Context context,
                         @Nullable String offerId,
+                        long coinLockedInMultiSig,
                         boolean segwit) {
+        this(keyPair.getPubKey(),
+                keyPair.getPubKeyHash(),
+                context,
+                offerId,
+                coinLockedInMultiSig,
+                segwit);
         this.keyPair = keyPair;
-        this.context = context;
-        this.offerId = offerId;
-        pubKey = keyPair.getPubKey();
-        pubKeyHash = keyPair.getPubKeyHash();
-        this.segwit = segwit;
     }
 
 
@@ -114,13 +130,13 @@ private AddressEntry(byte[] pubKey,
                          byte[] pubKeyHash,
                          Context context,
                          @Nullable String offerId,
-                         Coin coinLockedInMultiSig,
+                         long coinLockedInMultiSig,
                          boolean segwit) {
         this.pubKey = pubKey;
         this.pubKeyHash = pubKeyHash;
         this.context = context;
         this.offerId = offerId;
-        this.coinLockedInMultiSig = coinLockedInMultiSig.value;
+        this.coinLockedInMultiSig = coinLockedInMultiSig;
         this.segwit = segwit;
     }
 
@@ -129,7 +145,7 @@ public static AddressEntry fromProto(protobuf.AddressEntry proto) {
                 proto.getPubKeyHash().toByteArray(),
                 ProtoUtil.enumFromProto(AddressEntry.Context.class, proto.getContext().name()),
                 ProtoUtil.stringOrNullFromProto(proto.getOfferId()),
-                Coin.valueOf(proto.getCoinLockedInMultiSig()),
+                proto.getCoinLockedInMultiSig(),
                 proto.getSegwit());
     }
 
@@ -164,10 +180,6 @@ public DeterministicKey getKeyPair() {
         return keyPair;
     }
 
-    public void setCoinLockedInMultiSig(@NotNull Coin coinLockedInMultiSig) {
-        this.coinLockedInMultiSig = coinLockedInMultiSig.value;
-    }
-
     // For display we usually only display the first 8 characters.
     @Nullable
     public String getShortOfferId() {
@@ -183,11 +195,19 @@ public String getAddressString() {
 
     @Nullable
     public Address getAddress() {
-        if (address == null && keyPair != null)
+        if (address == null && keyPair != null) {
             address = Address.fromKey(Config.baseCurrencyNetworkParameters(), keyPair, segwit ? Script.ScriptType.P2WPKH : Script.ScriptType.P2PKH);
+        }
+        if (address == null) {
+            log.warn("Address is null at getAddress(). keyPair={}", keyPair);
+        }
         return address;
     }
 
+    public boolean isAddressNull() {
+        return address == null;
+    }
+
     public boolean isOpenOffer() {
         return context == Context.OFFER_FUNDING || context == Context.RESERVED_FOR_TRADE;
     }
@@ -196,7 +216,7 @@ public boolean isTrade() {
         return context == Context.MULTI_SIG || context == Context.TRADE_PAYOUT;
     }
 
-    public Coin getCoinLockedInMultiSig() {
+    public Coin getCoinLockedInMultiSigAsCoin() {
         return Coin.valueOf(coinLockedInMultiSig);
     }
 

core/src/main/java/bisq/core/btc/model/AddressEntryList.java

@@ -118,7 +118,7 @@ public void onWalletReady(Wallet wallet) {
                     Address addressFromKey = Address.fromKey(Config.baseCurrencyNetworkParameters(), keyFromPubHash,
                             scriptType);
                     // We want to ensure key and address matches in case we have address in entry available already
-                    if (addressEntry.getAddress() == null || addressFromKey.equals(addressEntry.getAddress())) {
+                    if (addressEntry.isAddressNull() || addressFromKey.equals(addressEntry.getAddress())) {
                         addressEntry.setDeterministicKey(keyFromPubHash);
                     } else {
                         log.error("We found an address entry without key but cannot apply the key as the address " +
@@ -149,11 +149,13 @@ public void onWalletReady(Wallet wallet) {
             wallet.getIssuedReceiveAddresses().stream()
                     .filter(this::isAddressNotInEntries)
                     .forEach(address -> {
-                        log.info("Create AddressEntry for IssuedReceiveAddress. address={}", address.toString());
-                        DeterministicKey key = (DeterministicKey) wallet.findKeyFromAddress(address);
+                         DeterministicKey key = (DeterministicKey) wallet.findKeyFromAddress(address);
                         if (key != null) {
                             // Address will be derived from key in getAddress method
+                            log.info("Create AddressEntry for IssuedReceiveAddress. address={}", address.toString());
                             entrySet.add(new AddressEntry(key, AddressEntry.Context.AVAILABLE, address instanceof SegwitAddress));
+                        } else {
+                            log.warn("DeterministicKey for address {} is null", address);
                         }
                     });
         }
@@ -190,12 +192,23 @@ public void addAddressEntry(AddressEntry addressEntry) {
             return;
         }
 
+        log.info("addAddressEntry: add new AddressEntry {}", addressEntry);
         boolean setChangedByAdd = entrySet.add(addressEntry);
         if (setChangedByAdd)
             requestPersistence();
     }
 
     public void swapToAvailable(AddressEntry addressEntry) {
+        if (addressEntry.getContext() == AddressEntry.Context.MULTI_SIG) {
+            log.error("swapToAvailable called with an addressEntry with MULTI_SIG context. " +
+                    "This in not permitted as we must not reuse those address entries and there are " +
+                    "no redeemable funds on those addresses. " +
+                    "Only the keys are used for creating the Multisig address. " +
+                    "addressEntry={}", addressEntry);
+            return;
+        }
+
+        log.info("swapToAvailable addressEntry to swap={}", addressEntry);
         boolean setChangedByRemove = entrySet.remove(addressEntry);
         boolean setChangedByAdd = entrySet.add(new AddressEntry(addressEntry.getKeyPair(),
                 AddressEntry.Context.AVAILABLE,
@@ -209,14 +222,34 @@ public AddressEntry swapAvailableToAddressEntryWithOfferId(AddressEntry addressE
                                                                AddressEntry.Context context,
                                                                String offerId) {
         boolean setChangedByRemove = entrySet.remove(addressEntry);
-        final AddressEntry newAddressEntry = new AddressEntry(addressEntry.getKeyPair(), context, offerId, addressEntry.isSegwit());
+        AddressEntry newAddressEntry = new AddressEntry(addressEntry.getKeyPair(), context, offerId, addressEntry.isSegwit());
+        log.info("swapAvailableToAddressEntryWithOfferId newAddressEntry={}", newAddressEntry);
         boolean setChangedByAdd = entrySet.add(newAddressEntry);
         if (setChangedByRemove || setChangedByAdd)
             requestPersistence();
 
         return newAddressEntry;
     }
 
+    public void setCoinLockedInMultiSigAddressEntry(AddressEntry addressEntry, long value) {
+        if (addressEntry.getContext() != AddressEntry.Context.MULTI_SIG) {
+            log.error("setCoinLockedInMultiSigAddressEntry must be called only on MULTI_SIG entries");
+            return;
+        }
+
+        log.info("setCoinLockedInMultiSigAddressEntry addressEntry={}, value={}", addressEntry, value);
+        boolean setChangedByRemove = entrySet.remove(addressEntry);
+        AddressEntry entry = new AddressEntry(addressEntry.getKeyPair(),
+                addressEntry.getContext(),
+                addressEntry.getOfferId(),
+                value,
+                addressEntry.isSegwit());
+        boolean setChangedByAdd = entrySet.add(entry);
+        if (setChangedByRemove || setChangedByAdd) {
+            requestPersistence();
+        }
+    }
+
     public void requestPersistence() {
         persistenceManager.requestPersistence();
     }

core/src/main/java/bisq/core/btc/setup/WalletConfig.java

@@ -265,7 +265,6 @@ protected void onSetupCompleted() {
     protected void startUp() throws Exception {
         // Runs in a separate thread.
         Context.propagate(context);
-        log.info("Starting up with directory = {}", directory);
         try {
             File chainFile = new File(directory, filePrefix + ".spvchain");
             boolean chainFileExists = chainFile.exists();

core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java

@@ -45,12 +45,12 @@
 import org.bitcoinj.core.LegacyAddress;
 import org.bitcoinj.core.NetworkParameters;
 import org.bitcoinj.core.Sha256Hash;
-import org.bitcoinj.script.ScriptException;
 import org.bitcoinj.core.Transaction;
 import org.bitcoinj.core.TransactionConfidence;
 import org.bitcoinj.core.TransactionInput;
 import org.bitcoinj.core.TransactionOutPoint;
 import org.bitcoinj.core.TransactionOutput;
+import org.bitcoinj.script.ScriptException;
 import org.bitcoinj.wallet.CoinSelection;
 import org.bitcoinj.wallet.CoinSelector;
 import org.bitcoinj.wallet.SendRequest;
@@ -565,6 +565,7 @@ private Transaction getPreparedSendTx(String receiverAddress, Coin receiverAmoun
 
             return tx;
         } catch (InsufficientMoneyException e) {
+            log.error("getPreparedSendTx: tx={}", tx.toString());
             log.error(e.toString());
             throw new InsufficientBsqException(e.missing);
         }

core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java

@@ -638,6 +638,7 @@ public AddressEntry getOrCreateAddressEntry(String offerId, AddressEntry.Context
             } else {
                 DeterministicKey key = (DeterministicKey) wallet.findKeyFromAddress(wallet.freshReceiveAddress(Script.ScriptType.P2WPKH));
                 AddressEntry entry = new AddressEntry(key, context, offerId, true);
+                log.info("getOrCreateAddressEntry: new AddressEntry={}", entry);
                 addressEntryList.addAddressEntry(entry);
                 return entry;
             }
@@ -689,6 +690,7 @@ private AddressEntry getOrCreateAddressEntry(AddressEntry.Context context,
                 key = (DeterministicKey) wallet.findKeyFromAddress(wallet.freshReceiveAddress(Script.ScriptType.P2PKH));
             }
             AddressEntry entry = new AddressEntry(key, context, segwit);
+            log.info("getOrCreateAddressEntry: add new AddressEntry {}", entry);
             addressEntryList.addAddressEntry(entry);
             return entry;
         }
@@ -738,6 +740,14 @@ public List<AddressEntry> getAddressEntryListAsImmutableList() {
     }
 
     public void swapTradeEntryToAvailableEntry(String offerId, AddressEntry.Context context) {
+        if (context == AddressEntry.Context.MULTI_SIG) {
+            log.error("swapTradeEntryToAvailableEntry called with MULTI_SIG context. " +
+                    "This in not permitted as we must not reuse those address entries and there " +
+                    "are no redeemable funds on that addresses. Only the keys are used for creating " +
+                    "the Multisig address. offerId={}, context={}", offerId, context);
+            return;
+        }
+
         getAddressEntryListAsImmutableList().stream()
                 .filter(e -> offerId.equals(e.getOfferId()))
                 .filter(e -> context == e.getContext())
@@ -748,15 +758,35 @@ public void swapTradeEntryToAvailableEntry(String offerId, AddressEntry.Context
                 });
     }
 
+    // When funds from MultiSig address is spent we reset the coinLockedInMultiSig value to 0.
+    public void resetCoinLockedInMultiSigAddressEntry(String offerId) {
+        setCoinLockedInMultiSigAddressEntry(offerId, 0);
+    }
+
+    public void setCoinLockedInMultiSigAddressEntry(String offerId, long value) {
+        getAddressEntryListAsImmutableList().stream()
+                .filter(e -> AddressEntry.Context.MULTI_SIG == e.getContext())
+                .filter(e -> offerId.equals(e.getOfferId()))
+                .forEach(addressEntry -> setCoinLockedInMultiSigAddressEntry(addressEntry, value));
+    }
+
+    public void setCoinLockedInMultiSigAddressEntry(AddressEntry addressEntry, long value) {
+        log.info("Set coinLockedInMultiSig for addressEntry {} to value {}", addressEntry, value);
+        addressEntryList.setCoinLockedInMultiSigAddressEntry(addressEntry, value);
+    }
+
     public void resetAddressEntriesForOpenOffer(String offerId) {
         log.info("resetAddressEntriesForOpenOffer offerId={}", offerId);
         swapTradeEntryToAvailableEntry(offerId, AddressEntry.Context.OFFER_FUNDING);
         swapTradeEntryToAvailableEntry(offerId, AddressEntry.Context.RESERVED_FOR_TRADE);
     }
 
     public void resetAddressEntriesForPendingTrade(String offerId) {
-        swapTradeEntryToAvailableEntry(offerId, AddressEntry.Context.MULTI_SIG);
-        // We swap also TRADE_PAYOUT to be sure all is cleaned up. There might be cases where a user cannot send the funds
+        // We must not swap MULTI_SIG entries as those addresses are not detected in the isAddressUnused
+        // check at getOrCreateAddressEntry and could lead to a reuse of those keys and result in the same 2of2 MS
+        // address if same peers trade again.
+
+        // We swap TRADE_PAYOUT to be sure all is cleaned up. There might be cases where a user cannot send the funds
         // to an external wallet directly in the last step of the trade, but the funds are in the Bisq wallet anyway and
         // the dealing with the external wallet is pure UI thing. The user can move the funds to the wallet and then
         // send out the funds to the external wallet. As this cleanup is a rare situation and most users do not use