[bisq-network/bisq] [WIP] Add list to filter to set btc fee receiver addresses (#4150)

Steven Barclay notifications at github.com
Sat May 16 10:09:10 UTC 2020


@stejbac commented on this pull request.

There are some missing changes to the unit tests `OpenOfferManagerTest` and `UserPayloadModelVOTest` which break the build. Other than that, I couldn't see any obvious problem when testing an Alice and Bob instance on regtest. (I also did some limited smoke testing on mainnet.)

I was not able to get the filter to propagate from one node to another on regtest (using `--useDevPrivilegeKeys`) and had to add them separately to each Alice/Bob/Mediator instance, though that almost certainly has nothing to do with these changes. I'm still looking into this - possibly intended behaviour of `FilterManager`?

> @@ -51,6 +52,8 @@
     private final TradeStatisticsManager tradeStatisticsManager;
     private final DaoFacade daoFacade;
     private final User user;
+    @Getter

The `@Getter` field annotation here is redundant, as the class is already annotated with `@Getter`.

> @@ -628,6 +628,7 @@ message Filter {
     string disable_trade_below_version = 16;
     repeated string mediators = 17;
     repeated string refundAgents = 18;
+    repeated string btc_fee_receiver_addresses = 19;

Note: There is a conflict when rebasing onto master, as there was recently a new Filter field added:
> repeated string bannedSignerPubKeys = 19;

> +
+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;
+
+import java.util.List;
+import java.util.Random;
+
+import lombok.extern.slf4j.Slf4j;
+
+ at Slf4j
+public class FeeReceiverSelector {
+    public static String getAddress(DaoFacade daoFacade, FilterManager filterManager) {

I also think it would be a good idea to add an end date to help prevent overpaying.

With a single end date but no weightings, it would have to be set fairly near in the future to avoid overpaying the smallest amount due, so it might make things easier to administer with weightings as well. That way all the addresses fill up at roughly the same time (though there may be significant random fluctuation in the time taken - perhaps 5-10% as a rough guess).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/4150#pullrequestreview-413061028
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20200516/a1fef395/attachment.html>


More information about the bisq-github mailing list