From e2bf77fb594da1d6342d7c1fe5764e8d37c98e9e Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 18 Oct 2021 22:16:49 +0200 Subject: [PATCH 1/3] Fixes https://github.com/bisq-network/bisq/issues/5762 Avoid that outdated donation and fee addresses are used. In case we get an outdated donation address (RECIPIENT_BTC_ADDRESS) we trigger a dao resync. The user get a popup for doing a restart in that case. For the fee address selection we do not fall back to the RECIPIENT_BTC_ADDRESS in case no address from filter is provided but we use the hard coded address of the current fee receiver address. --- .../dao/state/DaoStateSnapshotService.java | 20 ++++++++ .../bisq_v1/tasks/CreateMakerFeeTx.java | 2 +- .../trade/DelayedPayoutAddressProvider.java | 47 +++++++++++++++++++ .../bisq_v1/tasks/taker/CreateTakerFeeTx.java | 2 +- .../bisq/core/util/FeeReceiverSelector.java | 16 ++++--- .../core/util/FeeReceiverSelectorTest.java | 22 ++------- 6 files changed, 83 insertions(+), 26 deletions(-) create mode 100644 core/src/main/java/bisq/core/trade/DelayedPayoutAddressProvider.java diff --git a/core/src/main/java/bisq/core/dao/state/DaoStateSnapshotService.java b/core/src/main/java/bisq/core/dao/state/DaoStateSnapshotService.java index 734dd065c76..8d0dbab2814 100644 --- a/core/src/main/java/bisq/core/dao/state/DaoStateSnapshotService.java +++ b/core/src/main/java/bisq/core/dao/state/DaoStateSnapshotService.java @@ -18,11 +18,13 @@ package bisq.core.dao.state; import bisq.core.dao.DaoSetupService; +import bisq.core.dao.governance.param.Param; import bisq.core.dao.monitoring.DaoStateMonitoringService; import bisq.core.dao.monitoring.model.DaoStateHash; import bisq.core.dao.state.model.DaoState; import bisq.core.dao.state.model.blockchain.Block; import bisq.core.dao.state.storage.DaoStateStorageService; +import bisq.core.trade.DelayedPayoutAddressProvider; import bisq.core.user.Preferences; import bisq.common.config.Config; @@ -62,6 +64,7 @@ public class DaoStateSnapshotService implements DaoSetupService, DaoStateListene private final DaoStateStorageService daoStateStorageService; private final DaoStateMonitoringService daoStateMonitoringService; private final Preferences preferences; + private final Config config; private final File storageDir; private protobuf.DaoState daoStateCandidate; @@ -86,12 +89,14 @@ public DaoStateSnapshotService(DaoStateService daoStateService, DaoStateStorageService daoStateStorageService, DaoStateMonitoringService daoStateMonitoringService, Preferences preferences, + Config config, @Named(Config.STORAGE_DIR) File storageDir) { this.daoStateService = daoStateService; this.genesisTxInfo = genesisTxInfo; this.daoStateStorageService = daoStateStorageService; this.daoStateMonitoringService = daoStateMonitoringService; this.preferences = preferences; + this.config = config; this.storageDir = storageDir; } @@ -114,6 +119,20 @@ public void start() { // DaoStateListener /////////////////////////////////////////////////////////////////////////////////////////// + @Override + public void onParseBlockCompleteAfterBatchProcessing(Block block) { + if (config.baseCurrencyNetwork.isMainnet()) { + // In case the DAO state is invalid we might get an outdated RECIPIENT_BTC_ADDRESS. In that case we trigger + // a dao resync from resources. + String address = daoStateService.getParamValue(Param.RECIPIENT_BTC_ADDRESS, daoStateService.getChainHeight()); + if (DelayedPayoutAddressProvider.isOutdatedAddress(address)) { + log.warn("The RECIPIENT_BTC_ADDRESS is not as expected. The DAO state is probably out of " + + "sync and a resync should fix that issue."); + resyncDaoStateFromResources(); + } + } + } + // We listen onDaoStateChanged to ensure the dao state has been processed from listener clients after parsing. // We need to listen during batch processing as well to write snapshots during that process. @Override @@ -129,6 +148,7 @@ public void onDaoStateChanged(Block block) { } } + @Override public void onParseBlockChainComplete() { isParseBlockChainComplete = true; diff --git a/core/src/main/java/bisq/core/offer/placeoffer/bisq_v1/tasks/CreateMakerFeeTx.java b/core/src/main/java/bisq/core/offer/placeoffer/bisq_v1/tasks/CreateMakerFeeTx.java index eeb21bda5c7..471593a3d25 100644 --- a/core/src/main/java/bisq/core/offer/placeoffer/bisq_v1/tasks/CreateMakerFeeTx.java +++ b/core/src/main/java/bisq/core/offer/placeoffer/bisq_v1/tasks/CreateMakerFeeTx.java @@ -66,7 +66,7 @@ protected void run() { TradeWalletService tradeWalletService = model.getTradeWalletService(); - String feeReceiver = FeeReceiverSelector.getAddress(model.getDaoFacade(), model.getFilterManager()); + String feeReceiver = FeeReceiverSelector.getAddress(model.getFilterManager()); if (offer.isCurrencyForMakerFeeBtc()) { tradeWalletService.createBtcTradingFeeTx( diff --git a/core/src/main/java/bisq/core/trade/DelayedPayoutAddressProvider.java b/core/src/main/java/bisq/core/trade/DelayedPayoutAddressProvider.java new file mode 100644 index 00000000000..f31c313bc56 --- /dev/null +++ b/core/src/main/java/bisq/core/trade/DelayedPayoutAddressProvider.java @@ -0,0 +1,47 @@ +/* + * This file is part of Bisq. + * + * Bisq is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or (at + * your option) any later version. + * + * Bisq is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public + * License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with Bisq. If not, see . + */ + +package bisq.core.trade; + +import bisq.core.dao.DaoFacade; +import bisq.core.dao.governance.param.Param; + +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class DelayedPayoutAddressProvider { + public static String getDelayedPayoutAddress(DaoFacade daoFacade) { + String address = daoFacade.getParamValue(Param.RECIPIENT_BTC_ADDRESS); + if (isOutdatedAddress(address)) { + log.warn("Outdated delayed payout address. " + + "This can be the case if the DAO is deactivated or if the user has an invalid DAO state." + + "We set the address to the recent one (34VLFgtFKAtwTdZ5rengTT2g2zC99sWQLC)."); + return getAddress(); + } + return address; + } + + public static boolean isOutdatedAddress(String address) { + return address.equals("1BVxNn3T12veSK6DgqwU4Hdn7QHcDDRag7") || // Initial DAO donation address + address.equals("3EtUWqsGThPtjwUczw27YCo6EWvQdaPUyp") || // burning2019 + address.equals("3A8Zc1XioE2HRzYfbb5P8iemCS72M6vRJV"); // burningman2 + } + + public static String getAddress() { + return "34VLFgtFKAtwTdZ5rengTT2g2zC99sWQLC"; + } +} diff --git a/core/src/main/java/bisq/core/trade/protocol/bisq_v1/tasks/taker/CreateTakerFeeTx.java b/core/src/main/java/bisq/core/trade/protocol/bisq_v1/tasks/taker/CreateTakerFeeTx.java index b03817ab9bb..327177aae28 100644 --- a/core/src/main/java/bisq/core/trade/protocol/bisq_v1/tasks/taker/CreateTakerFeeTx.java +++ b/core/src/main/java/bisq/core/trade/protocol/bisq_v1/tasks/taker/CreateTakerFeeTx.java @@ -65,7 +65,7 @@ protected void run() { TradeWalletService tradeWalletService = processModel.getTradeWalletService(); Transaction transaction; - String feeReceiver = FeeReceiverSelector.getAddress(processModel.getDaoFacade(), processModel.getFilterManager()); + String feeReceiver = FeeReceiverSelector.getAddress(processModel.getFilterManager()); if (trade.isCurrencyForTakerFeeBtc()) { transaction = tradeWalletService.createBtcTradingFeeTx( diff --git a/core/src/main/java/bisq/core/util/FeeReceiverSelector.java b/core/src/main/java/bisq/core/util/FeeReceiverSelector.java index 2854a0e09f1..252275d642c 100644 --- a/core/src/main/java/bisq/core/util/FeeReceiverSelector.java +++ b/core/src/main/java/bisq/core/util/FeeReceiverSelector.java @@ -17,8 +17,6 @@ package bisq.core.util; -import bisq.core.dao.DaoFacade; -import bisq.core.dao.governance.param.Param; import bisq.core.filter.FilterManager; import org.bitcoinj.core.Coin; @@ -34,12 +32,16 @@ @Slf4j public class FeeReceiverSelector { - public static String getAddress(DaoFacade daoFacade, FilterManager filterManager) { - return getAddress(daoFacade, filterManager, new Random()); + public static String getMostRecentAddress() { + return "38bZBj5peYS3Husdz7AH3gEUiUbYRD951t"; + } + + public static String getAddress(FilterManager filterManager) { + return getAddress(filterManager, new Random()); } @VisibleForTesting - static String getAddress(DaoFacade daoFacade, FilterManager filterManager, Random rnd) { + static String getAddress(FilterManager filterManager, Random rnd) { List feeReceivers = Optional.ofNullable(filterManager.getFilter()) .flatMap(f -> Optional.ofNullable(f.getBtcFeeReceiverAddresses())) .orElse(List.of()); @@ -61,8 +63,8 @@ static String getAddress(DaoFacade daoFacade, FilterManager filterManager, Rando return receiverAddressList.get(weightedSelection(amountList, rnd)); } - // We keep default value as fallback in case no filter value is available or user has old version. - return daoFacade.getParamValue(Param.RECIPIENT_BTC_ADDRESS); + // If no fee address receiver is defined via filter we use the hard coded recent address + return getMostRecentAddress(); } @VisibleForTesting diff --git a/core/src/test/java/bisq/core/util/FeeReceiverSelectorTest.java b/core/src/test/java/bisq/core/util/FeeReceiverSelectorTest.java index a5632e7207e..4d38a1487a2 100644 --- a/core/src/test/java/bisq/core/util/FeeReceiverSelectorTest.java +++ b/core/src/test/java/bisq/core/util/FeeReceiverSelectorTest.java @@ -17,8 +17,6 @@ package bisq.core.util; -import bisq.core.dao.DaoFacade; -import bisq.core.dao.governance.param.Param; import bisq.core.filter.Filter; import bisq.core.filter.FilterManager; @@ -42,8 +40,6 @@ @RunWith(MockitoJUnitRunner.class) public class FeeReceiverSelectorTest { - @Mock - private DaoFacade daoFacade; @Mock private FilterManager filterManager; @@ -55,7 +51,7 @@ public void testGetAddress() { Map selectionCounts = new HashMap<>(); for (int i = 0; i < 400; i++) { - String address = FeeReceiverSelector.getAddress(daoFacade, filterManager, rnd); + String address = FeeReceiverSelector.getAddress(filterManager, rnd); selectionCounts.compute(address, (k, n) -> n != null ? n + 1 : 1); } @@ -69,34 +65,26 @@ public void testGetAddress() { @Test public void testGetAddress_noValidReceivers_nullFilter() { - when(daoFacade.getParamValue(Param.RECIPIENT_BTC_ADDRESS)).thenReturn("default"); - when(filterManager.getFilter()).thenReturn(null); - assertEquals("default", FeeReceiverSelector.getAddress(daoFacade, filterManager)); + assertEquals(FeeReceiverSelector.getMostRecentAddress(), FeeReceiverSelector.getAddress(filterManager)); } @Test public void testGetAddress_noValidReceivers_filterWithNullList() { - when(daoFacade.getParamValue(Param.RECIPIENT_BTC_ADDRESS)).thenReturn("default"); - when(filterManager.getFilter()).thenReturn(filterWithReceivers(null)); - assertEquals("default", FeeReceiverSelector.getAddress(daoFacade, filterManager)); + assertEquals(FeeReceiverSelector.getMostRecentAddress(), FeeReceiverSelector.getAddress(filterManager)); } @Test public void testGetAddress_noValidReceivers_filterWithEmptyList() { - when(daoFacade.getParamValue(Param.RECIPIENT_BTC_ADDRESS)).thenReturn("default"); - when(filterManager.getFilter()).thenReturn(filterWithReceivers(List.of())); - assertEquals("default", FeeReceiverSelector.getAddress(daoFacade, filterManager)); + assertEquals(FeeReceiverSelector.getMostRecentAddress(), FeeReceiverSelector.getAddress(filterManager)); } @Test public void testGetAddress_noValidReceivers_filterWithIllFormedList() { - when(daoFacade.getParamValue(Param.RECIPIENT_BTC_ADDRESS)).thenReturn("default"); - when(filterManager.getFilter()).thenReturn(filterWithReceivers(List.of("ill-formed"))); - assertEquals("default", FeeReceiverSelector.getAddress(daoFacade, filterManager)); + assertEquals(FeeReceiverSelector.getMostRecentAddress(), FeeReceiverSelector.getAddress(filterManager)); } @Test From beb4590ef7bfb9cf8f1f09e9e52800b330ed3bc9 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Tue, 9 Nov 2021 20:31:52 +0100 Subject: [PATCH 2/3] Use static fields for addresses --- core/src/main/java/bisq/core/dao/DaoFacade.java | 7 ++++--- .../core/dao/state/DaoStateSnapshotService.java | 1 - .../core/trade/DelayedPayoutAddressProvider.java | 16 +++++++++++----- .../java/bisq/core/util/FeeReceiverSelector.java | 4 +++- .../core/provider/mempool/TxValidatorTest.java | 14 ++++++++------ 5 files changed, 26 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/bisq/core/dao/DaoFacade.java b/core/src/main/java/bisq/core/dao/DaoFacade.java index 75c44037cdb..082472e3d9f 100644 --- a/core/src/main/java/bisq/core/dao/DaoFacade.java +++ b/core/src/main/java/bisq/core/dao/DaoFacade.java @@ -73,6 +73,7 @@ import bisq.core.dao.state.model.governance.RoleProposal; import bisq.core.dao.state.model.governance.Vote; import bisq.core.dao.state.storage.DaoStateStorageService; +import bisq.core.trade.DelayedPayoutAddressProvider; import bisq.asset.Asset; @@ -799,9 +800,9 @@ public Set getAllDonationAddresses() { if (Config.baseCurrencyNetwork().isMainnet()) { // If Dao is deactivated we need to add the past addresses used as well. // This list need to be updated once a new address gets defined. - allPastParamValues.add("3EtUWqsGThPtjwUczw27YCo6EWvQdaPUyp"); // burning man 2019 - allPastParamValues.add("3A8Zc1XioE2HRzYfbb5P8iemCS72M6vRJV"); // burningman2 - allPastParamValues.add("34VLFgtFKAtwTdZ5rengTT2g2zC99sWQLC"); // burningman3 (https://github.com/bisq-network/roles/issues/80#issuecomment-723577776) + allPastParamValues.add(DelayedPayoutAddressProvider.BM2019_ADDRESS); + allPastParamValues.add(DelayedPayoutAddressProvider.BM2_ADDRESS); + allPastParamValues.add(DelayedPayoutAddressProvider.BM3_ADDRESS); } return allPastParamValues; diff --git a/core/src/main/java/bisq/core/dao/state/DaoStateSnapshotService.java b/core/src/main/java/bisq/core/dao/state/DaoStateSnapshotService.java index 8d0dbab2814..877880be651 100644 --- a/core/src/main/java/bisq/core/dao/state/DaoStateSnapshotService.java +++ b/core/src/main/java/bisq/core/dao/state/DaoStateSnapshotService.java @@ -148,7 +148,6 @@ public void onDaoStateChanged(Block block) { } } - @Override public void onParseBlockChainComplete() { isParseBlockChainComplete = true; diff --git a/core/src/main/java/bisq/core/trade/DelayedPayoutAddressProvider.java b/core/src/main/java/bisq/core/trade/DelayedPayoutAddressProvider.java index f31c313bc56..c05482939c3 100644 --- a/core/src/main/java/bisq/core/trade/DelayedPayoutAddressProvider.java +++ b/core/src/main/java/bisq/core/trade/DelayedPayoutAddressProvider.java @@ -24,24 +24,30 @@ @Slf4j public class DelayedPayoutAddressProvider { + public static final String INITIAL_BM_ADDRESS = "1BVxNn3T12veSK6DgqwU4Hdn7QHcDDRag7"; // Initial DAO donation address + public static final String BM2019_ADDRESS = "3EtUWqsGThPtjwUczw27YCo6EWvQdaPUyp"; // burning2019 + public static final String BM2_ADDRESS = "3A8Zc1XioE2HRzYfbb5P8iemCS72M6vRJV"; // burningman2 + // burningman3 https://github.com/bisq-network/roles/issues/80#issuecomment-723577776 + public static final String BM3_ADDRESS = "34VLFgtFKAtwTdZ5rengTT2g2zC99sWQLC"; + public static String getDelayedPayoutAddress(DaoFacade daoFacade) { String address = daoFacade.getParamValue(Param.RECIPIENT_BTC_ADDRESS); if (isOutdatedAddress(address)) { log.warn("Outdated delayed payout address. " + "This can be the case if the DAO is deactivated or if the user has an invalid DAO state." + - "We set the address to the recent one (34VLFgtFKAtwTdZ5rengTT2g2zC99sWQLC)."); + "We set the address to the recent one (BM3_ADDRESS)."); return getAddress(); } return address; } public static boolean isOutdatedAddress(String address) { - return address.equals("1BVxNn3T12veSK6DgqwU4Hdn7QHcDDRag7") || // Initial DAO donation address - address.equals("3EtUWqsGThPtjwUczw27YCo6EWvQdaPUyp") || // burning2019 - address.equals("3A8Zc1XioE2HRzYfbb5P8iemCS72M6vRJV"); // burningman2 + return address.equals(INITIAL_BM_ADDRESS) || + address.equals(BM2019_ADDRESS) || + address.equals(BM2_ADDRESS); } public static String getAddress() { - return "34VLFgtFKAtwTdZ5rengTT2g2zC99sWQLC"; + return BM3_ADDRESS; } } diff --git a/core/src/main/java/bisq/core/util/FeeReceiverSelector.java b/core/src/main/java/bisq/core/util/FeeReceiverSelector.java index 252275d642c..9e093333561 100644 --- a/core/src/main/java/bisq/core/util/FeeReceiverSelector.java +++ b/core/src/main/java/bisq/core/util/FeeReceiverSelector.java @@ -32,8 +32,10 @@ @Slf4j public class FeeReceiverSelector { + public static final String BTC_FEE_RECEIVER_ADDRESS = "38bZBj5peYS3Husdz7AH3gEUiUbYRD951t"; + public static String getMostRecentAddress() { - return "38bZBj5peYS3Husdz7AH3gEUiUbYRD951t"; + return BTC_FEE_RECEIVER_ADDRESS; } public static String getAddress(FilterManager filterManager) { diff --git a/core/src/test/java/bisq/core/provider/mempool/TxValidatorTest.java b/core/src/test/java/bisq/core/provider/mempool/TxValidatorTest.java index 7b546af6984..d3c68c61456 100644 --- a/core/src/test/java/bisq/core/provider/mempool/TxValidatorTest.java +++ b/core/src/test/java/bisq/core/provider/mempool/TxValidatorTest.java @@ -19,15 +19,17 @@ import bisq.core.dao.governance.param.Param; import bisq.core.dao.state.DaoStateService; +import bisq.core.trade.DelayedPayoutAddressProvider; +import bisq.core.util.FeeReceiverSelector; import bisq.core.util.ParsingUtils; import bisq.core.util.coin.BsqFormatter; +import org.bitcoinj.core.Coin; + import com.google.gson.Gson; import org.apache.commons.io.IOUtils; -import org.bitcoinj.core.Coin; - import java.io.IOException; import java.util.ArrayList; @@ -43,8 +45,8 @@ import org.mockito.Mockito; import org.mockito.stubbing.Answer; -import org.junit.Test; import org.junit.Assert; +import org.junit.Test; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -61,11 +63,11 @@ public TxValidatorTest() { btcFeeReceivers.add("13sxMq8mTw7CTSqgGiMPfwo6ZDsVYrHLmR"); btcFeeReceivers.add("19qA2BVPoyXDfHKVMovKG7SoxGY7xrBV8c"); btcFeeReceivers.add("19BNi5EpZhgBBWAt5ka7xWpJpX2ZWJEYyq"); - btcFeeReceivers.add("38bZBj5peYS3Husdz7AH3gEUiUbYRD951t"); - btcFeeReceivers.add("3EtUWqsGThPtjwUczw27YCo6EWvQdaPUyp"); + btcFeeReceivers.add(FeeReceiverSelector.BTC_FEE_RECEIVER_ADDRESS); + btcFeeReceivers.add(DelayedPayoutAddressProvider.BM2019_ADDRESS); btcFeeReceivers.add("1BVxNn3T12veSK6DgqwU4Hdn7QHcDDRag7"); btcFeeReceivers.add("3A8Zc1XioE2HRzYfbb5P8iemCS72M6vRJV"); - btcFeeReceivers.add("34VLFgtFKAtwTdZ5rengTT2g2zC99sWQLC"); + btcFeeReceivers.add(DelayedPayoutAddressProvider.BM3_ADDRESS); log.warn("Known BTC fee receivers: {}", btcFeeReceivers.toString()); } From 8d22904ee388787b4b0a98cefefb6e4384ace4e6 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Tue, 9 Nov 2021 20:48:17 +0100 Subject: [PATCH 3/3] Fix missing param in test --- .../java/bisq/core/dao/state/DaoStateSnapshotServiceTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/test/java/bisq/core/dao/state/DaoStateSnapshotServiceTest.java b/core/src/test/java/bisq/core/dao/state/DaoStateSnapshotServiceTest.java index 5e8ccefe05b..db4839390e9 100644 --- a/core/src/test/java/bisq/core/dao/state/DaoStateSnapshotServiceTest.java +++ b/core/src/test/java/bisq/core/dao/state/DaoStateSnapshotServiceTest.java @@ -39,6 +39,7 @@ public void setup() { mock(DaoStateStorageService.class), mock(DaoStateMonitoringService.class), null, + null, null); }