[Qt] allow banning and unbanning over UI->peers table #6315

pull jonasschnelli wants to merge 20 commits into bitcoin:master from jonasschnelli:2015/06/qt_ban changing 14 files +654 −56
  1. jonasschnelli commented at 7:59 PM on June 20, 2015: contributor

    This adds 4 ban options to the peers tables context menu (1h, 24h, 7d, 365d). If there are active bans, a table with banned subnets will be shown. Sync between the rpc bans and the ui bans are guaranteed over a new uiInterface signal.

    Screenshots: bildschirmfoto-2015-06-20-um-21 58 44 bildschirmfoto-2015-06-20-um-21 58 59

  2. in src/rpcnet.cpp:None in 2233f12a56 outdated 
     527 | @@ -527,6 +528,7 @@ UniValue setban(const UniValue& params, bool fHelp)
 528 |              throw JSONRPCError(RPC_MISC_ERROR, "Error: Unban failed");
 529 |      }
 530 |  
 531 | +    uiInterface.BannedListChanged();
    jonasschnelli commented at 8:01 PM on June 20, 2015:

    I'm not sure if the locking of a signal emit is done within boost::signal of if this needs explicit locking of uiInterface?

    Diapolo commented at 3:31 PM on June 23, 2015:

    Perhaps @laanwj can comment on this?

  3. jonasschnelli force-pushed on Jun 20, 2015
  4. in src/qt/bantablemodel.cpp:None in b5621faf90 outdated 
     118 | +        case Address:
 119 | +            return QString::fromStdString(rec->subnet.ToString());
 120 | +        case Bantime:
 121 | +            //show time in users local timezone, not 64bit compatible!
 122 | +            //TODO find a way to support 64bit timestamps
 123 | +            boost::posix_time::ptime pt1 = boost::posix_time::from_time_t(rec->bantil);
    jonasschnelli commented at 8:03 PM on June 20, 2015:

    i recognized that boost::posix_time::from_time_t is not 64bit capable. Maybe someone has a idea how to fix this.

    Diapolo commented at 10:28 PM on June 20, 2015:

    What about QDateTime::fromTime_t()?

    QDateTime::fromTime_t(rec->bantil).toString() didn't test it...

    jonasschnelli commented at 8:10 AM on June 21, 2015:

    I think this won't work either. The docs are saying: QDateTime QDateTime::fromTime_t(uint seconds) (uint!).

    jonasschnelli commented at 8:25 AM on June 21, 2015:

    Found a fix! Pushed.

  5. in src/qt/bantablemodel.cpp:None in b5621faf90 outdated 
      31 | +    Qt::SortOrder sortOrder;
  32 | +
  33 | +    /** Pull a full list of banned nodes from CNode into our cache */
  34 | +    void refreshBanlist()
  35 | +    {
  36 | +        std::map<CSubNet, int64_t> banMap;
    Diapolo commented at 10:15 PM on June 20, 2015:

    I was thinking about a typedef for this, what do you mean?

    jonasschnelli commented at 8:46 AM on June 21, 2015:

    typedefs are useful but can also make reading the code more complicate. In this case (simple map) i don't see a usecase for a typedef.

    penguin1333 commented at 5:47 PM on June 23, 2015:

    Why would it make it more complicated? Worse comes to worse, you comment it, which is not really needed anyways.

  6. in src/qt/bantablemodel.cpp:None in b5621faf90 outdated 
      39 | +        cachedBanlist.clear();
  40 | +#if QT_VERSION >= 0x040700
  41 | +        cachedBanlist.reserve(banMap.size());
  42 | +#endif
  43 | +        std::map<CSubNet, int64_t>::iterator iter;
  44 | +        for (iter = banMap.begin(); iter != banMap.end(); ++iter) {
    Diapolo commented at 10:16 PM on June 20, 2015:

    Could this be a Qt foreach? Would also be easy to convert to C++11 then :)?

  7. in src/qt/bantablemodel.cpp:None in b5621faf90 outdated 
      47 | +            banEntry.bantil = iter->second;
  48 | +            cachedBanlist.append(banEntry);
  49 | +        }
  50 | +    }
  51 | +
  52 | +    int size()
    Diapolo commented at 10:17 PM on June 20, 2015:

    Could this be int size() const?

  8. in src/qt/bantablemodel.cpp:None in b5621faf90 outdated 
      56 | +
  57 | +    CCombinedBan *index(int idx)
  58 | +    {
  59 | +        if(idx >= 0 && idx < cachedBanlist.size()) {
  60 | +            return &cachedBanlist[idx];
  61 | +        } else {
    Diapolo commented at 10:26 PM on June 20, 2015:

    That else can be removed. I see you copied most of the code here from peertablemodel.cpp where all these suggestions would also fit and perhaps we should clean it up, to keep the codebase close together?

    jonasschnelli commented at 8:37 AM on June 21, 2015:

    Not sure if this else can be removed. What if the QTableView requests a row at index which is not in the cache QList. Obviously it should not happen because the table size is driven by the size of the cache QList. But I prefer keeping this.

    Diapolo commented at 11:00 AM on June 21, 2015:

    Not sure if you're right here, when the if clause isn't true we always run into the else (as we have a return in the if we won't reach the return 0, if the clause is false). I'd rather remove it...

  9. in src/qt/bantablemodel.cpp:None in b5621faf90 outdated 
     126 | +            ss << pt2;
 127 | +            return QString::fromStdString(ss.str());
 128 | +        }
 129 | +    } else if (role == Qt::TextAlignmentRole) {
 130 | +        if (index.column() == Bantime)
 131 | +            return (int)(Qt::AlignRight | Qt::AlignVCenter);
    Diapolo commented at 10:31 PM on June 20, 2015:

    Is int the right type here? I tried (QVariant) which also works.

  10. in src/qt/bantablemodel.cpp:None in b5621faf90 outdated 
     162 | +
 163 | +    if (data)
 164 | +    {
 165 | +        return createIndex(row, column, data);
 166 | +    }
 167 | +    else
    Diapolo commented at 10:32 PM on June 20, 2015:

    Same as above unneeded else. If we don't have data we can return just QModelIndex(). I think it's easier to read without else.

  11. in src/qt/bantablemodel.h:None in b5621faf90 outdated 
      16 | +
  17 | +QT_BEGIN_NAMESPACE
  18 | +class QTimer;
  19 | +QT_END_NAMESPACE
  20 | +
  21 | +struct CCombinedBan {
    Diapolo commented at 10:38 PM on June 20, 2015:

    We have a struct here and a std::map for similar stuff... is there a way to unify these 2?

    jonasschnelli commented at 8:45 AM on June 21, 2015:

    I think they should be separated and represent cores and ui form of keeping the data.

  12. in src/qt/bantablemodel.cpp:None in b5621faf90 outdated 
       6 | +
   7 | +#include "clientmodel.h"
   8 | +#include "guiconstants.h"
   9 | +#include "guiutil.h"
  10 | +
  11 | +#include "net.h"
    Diapolo commented at 10:38 PM on June 20, 2015:

    This is included in the header already.

  13. in src/qt/rpcconsole.cpp:None in b5621faf90 outdated 
     312 | @@ -311,23 +313,66 @@ void RPCConsole::setClientModel(ClientModel *model)
 313 |          ui->peerWidget->setColumnWidth(PeerTableModel::Address, ADDRESS_COLUMN_WIDTH);
 314 |          ui->peerWidget->setColumnWidth(PeerTableModel::Subversion, SUBVERSION_COLUMN_WIDTH);
 315 |          ui->peerWidget->setColumnWidth(PeerTableModel::Ping, PING_COLUMN_WIDTH);
 316 | +        ui->peerWidget->horizontalHeader()->setStretchLastSection(true);
 317 |  
 318 |          // create context menu actions
 319 | -        QAction* disconnectAction = new QAction(tr("&Disconnect Node"), this);
 320 | +        QAction* disconnectAction   = new QAction(tr("&Disconnect Node"), this);
 321 | +        QAction* banAction1h        = new QAction(tr("&Ban Node for 1 hour"), this);
    Diapolo commented at 10:43 PM on June 20, 2015:

    You are repeating some translation text, perhaps there is an efficient way to reduce that?

    jonasschnelli commented at 8:48 AM on June 21, 2015:

    split into two strings to reduce translation effort.

  14. in src/qt/rpcconsole.cpp:None in b5621faf90 outdated 
     757 | +        std::string addr;
 758 | +        int port = 0;
 759 | +        SplitHostPort(nStr, port, addr);
 760 | +
 761 | +        CNode::Ban(CNetAddr(addr), bantime);
 762 | +        bannedNode->CloseSocketDisconnect();
    Diapolo commented at 10:46 PM on June 20, 2015:

    Seems you reintroduced that over fDisconnect flag?

    jonasschnelli commented at 8:47 AM on June 21, 2015:

    This was a serious overlook. Fixed. Thanks.

  15. in src/qt/rpcconsole.cpp:None in b5621faf90 outdated 
     787 | @@ -699,3 +788,10 @@ void RPCConsole::clearSelectedNode()
 788 |      ui->detailWidget->hide();
 789 |      ui->peerHeading->setText(tr("Select a peer to view detailed information."));
 790 |  }
 791 | +
 792 | +void RPCConsole::showOrHideBanTableIfRequired()
 793 | +{
 794 | +    bool visible = clientModel->getBanTableModel()->shouldShow();
    Diapolo commented at 10:47 PM on June 20, 2015:

    This could also use a NULL pointer check for clientModel IMHO.

    jonasschnelli commented at 8:47 AM on June 21, 2015:

    Nullpointer check added.

  16. Diapolo commented at 10:49 PM on June 20, 2015: none

    Great pull IMHO, sorry for hitting you hard with that bunch of comments...

  17. jonasschnelli commented at 6:57 AM on June 21, 2015: contributor

    Thanks @Diapolo for the review. Appricate detailed feedback. Will fix all your points (especially the miss transaction to fDisconnect!).

  18. jonasschnelli commented at 10:45 AM on June 21, 2015: contributor

    Prebuilt binaries: https://builds.jonasschnelli.ch/pulls/6315/

  19. in src/qt/bantablemodel.cpp:None in 114c72213d outdated 
     122 | +            date = date.addSecs(rec->bantil);
 123 | +            return date.toString(Qt::SystemLocaleLongDate);
 124 | +        }
 125 | +    } else if (role == Qt::TextAlignmentRole) {
 126 | +        if (index.column() == Bantime)
 127 | +            return (int)(Qt::AlignRight | Qt::AlignVCenter);
    Diapolo commented at 11:02 AM on June 21, 2015:

    Still the question: Is int the right type here? I tried (QVariant) which also works and is more the Qt style?

    jonasschnelli commented at 11:03 AM on June 21, 2015:

    Right. This is also the same as we have it in PeersTableModel, it works, but i agree that it should be QVariant (as the method definition says).

    Diapolo commented at 11:12 AM on June 21, 2015:

    I opened #6317 and will try to keep that in sync with what we are doing here :).

  20. in src/qt/clientmodel.cpp:None in 114c72213d outdated 
     239 | @@ -226,12 +240,19 @@ static void NotifyAlertChanged(ClientModel *clientmodel, const uint256 &hash, Ch
 240 |                                Q_ARG(int, status));
 241 |  }
 242 |  
 243 | +static void BannedListChanged(ClientModel *clientmodel)
 244 | +{
 245 | +    qDebug() << "BannedListChanged";
    Diapolo commented at 11:21 AM on June 21, 2015:

    You could also use something like: qDebug() << QString("%1: Requesting update for peer banlist").arg(__func__); I intend to creat a pull to make use of that format so we have the debug logging include always the correct function name :).

  21. in src/qt/rpcconsole.cpp:None in 114c72213d outdated 
       7 | @@ -8,6 +8,7 @@
   8 |  #include "clientmodel.h"
   9 |  #include "guiutil.h"
  10 |  #include "peertablemodel.h"
  11 | +#include "bantablemodel.h"
    Diapolo commented at 11:24 AM on June 21, 2015:

    Not even a nit, but a please: Can you add #include "bantablemodel.h" above clientmodel :)?

  22. in src/qt/rpcconsole.cpp:None in 114c72213d outdated 
     336 |          // context menu signals
 337 | -        connect(ui->peerWidget, SIGNAL(customContextMenuRequested(const QPoint&)), this, SLOT(showMenu(const QPoint&)));
 338 | +        connect(ui->peerWidget, SIGNAL(customContextMenuRequested(const QPoint&)), this, SLOT(showPeersTableContextMenu(const QPoint&)));
 339 |          connect(disconnectAction, SIGNAL(triggered()), this, SLOT(disconnectSelectedNode()));
 340 |  
 341 | +        //add a signal mapping to allow a dynamic argument
    Diapolo commented at 11:25 AM on June 21, 2015:

    Nit: These comments are missing a whitespace at the front.

  23. in src/qt/rpcconsole.cpp:None in 114c72213d outdated 
     379 | @@ -335,6 +380,9 @@ void RPCConsole::setClientModel(ClientModel *model)
 380 |          ui->startupTime->setText(model->formatClientStartupTime());
 381 |  
 382 |          ui->networkName->setText(QString::fromStdString(Params().NetworkIDString()));
 383 | +
 384 | +        connect(model, SIGNAL(banListChanged()), this, SLOT(showOrHideBanTableIfRequired()));
    Diapolo commented at 11:30 AM on June 21, 2015:

    I'd rather group these near the banlist stuff or add some small comment, before this is getting confusing here?

  24. Diapolo commented at 11:31 AM on June 21, 2015: none

    Currently integrating this into my build to test, will report back :).

  25. Diapolo commented at 11:54 AM on June 21, 2015: none

    Observations:

    1. Banning a node, selecting a new node (from connected peers) and then selecting a banned node leaves the details of the previous (not banned) node visible in the right area.
    2. Banning a node selects the first cell in the banlist table without selecting the whole line and without a focus (cell is not marked active). I'd suggest the banned node should be selected and active entirely.
    3. I'm using Tor and hidden-service addresses + netmask + banned until are that large that I'm unable to view the whole values with the default sizes.
    4. The banned peers list has a heading Banned peers, perhaps the connected peers also should get a heading? I'd vote for a much smaller font size, the current one looks rather too big.

    Edit:

    1. There is a serious issue with banning incomming connections via Tor, these are all shown as 127.0.0.1 and handling them seems broken.
  26. Diapolo commented at 12:57 PM on June 21, 2015: none

    There is also an issue with removing a peer from banned peers list when banned until is over: ban

    The peer should be removed from the list, which is not working currently.

    listbanned also still shows them: [ { "address": "127.0.0.1/255.255.255.255", "banned_untill": 1434891295 }, { "address": "nkf5e6b7pl4jfd4a.onion/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "banned_untill": 1434891291 }, { "address": "t6xj6wilh4ytvcs7.onion/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "banned_untill": 1434891400 } ]

  27. jonasschnelli commented at 1:14 PM on June 21, 2015: contributor

    @Diapolo: for autocleaning then ban list see PR #6310

    I think you point 1) is not a problem. The peer is still selected (gray) when you interact with the bantable. Point 2) and 3) are cosmetics and should not hold this PR back. Point 4) agreed.

    Banning tor nodes needs testing (best first over RPC) and maybe needs more implementation. Would be good if you could see what's missing there.

  28. jonasschnelli commented at 2:02 PM on June 21, 2015: contributor

    Tested also with Tor nodes. Works fine. @Diapolo: what issues did you encounter with tor nodes?

  29. laanwj added the label GUI on Jun 22, 2015
  30. Diapolo commented at 2:18 PM on June 22, 2015: none

    @jonasschnelli The problem are incoming Tor nodes with 127.0.0.1 blocking one connected node blocks any other incoming 127.0.0.1 node. Other connected 127.0.0.1 are not affected.

    Also I'd suggest splitting of the autocleaning pull from the disk-store-pull as autocleaning is uncontroversial and also needed by this one.

    I also disagree on ignoring UI or UX issues for now as often such "small" or "minor" things are just forgotton or I pick them up. I'd vote for making this pull good and not just "working okay" :).

  31. jonasschnelli force-pushed on Jun 22, 2015
  32. jonasschnelli commented at 3:39 PM on June 22, 2015: contributor

    Fixed @Diapolos nits.

  33. luke-jr commented at 6:48 AM on June 23, 2015: member

    Can the netmask be changed to the more modern (eg) /24 /128 etc format? Also prefer to have banned peers not take up space in the peer tab... somehow.

  34. jonasschnelli force-pushed on Jun 23, 2015
  35. jonasschnelli commented at 3:21 PM on June 23, 2015: contributor

    Updated. Added a commit that uses CIDR notation (like /32) when printing (ToString()) CSubNet.

    UI and RPC looks now like... bildschirmfoto 2015-06-23 um 17 19 17 bildschirmfoto 2015-06-23 um 17 19 21

  36. Diapolo commented at 3:30 PM on June 23, 2015: none

    @jonasschnelli Thanks for your hard work on this, still there is a second round of nits open I made and also still the UX problems, the autoupdate (I have comments in the other pull also, so I'm fine if that get's some love or merged soon :)) and 127.0.0.1 handling for incoming Tor.

  37. jonasschnelli force-pushed on Jun 23, 2015
  38. jonasschnelli commented at 3:39 PM on June 23, 2015: contributor

    @Diapolo: what nits are open? Font size, selecting rows, column sized are fixed. Autocleanup is not in this PR and it will stay in #6310 because it makes most sense to clean data before writing to disk. It's not a bug if your ban list keeps a entry which has expired. Feel free to write a patch/commit for the incoming tor node banning (separate bans by port). I won't cover it in this PR.

  39. Diapolo commented at 3:42 PM on June 23, 2015: none

    @jonasschnelli Last time I saw quite some uncommented stuff in the code, even if minor. I need to resync from your repo and take another look then.

  40. jonasschnelli force-pushed on Jun 23, 2015
  41. jonasschnelli commented at 7:37 PM on June 23, 2015: contributor

    Added @Diapolos polishing commit on top.

  42. Diapolo commented at 8:08 PM on June 23, 2015: none

    Great, if I find the time I'll further investigate the Tor handling and also re-check the UX problems after I #6310 is merged (or after I integrated it into my build).

  43. Diapolo commented at 9:45 PM on June 23, 2015: none

    I've got another pull in the pipe covering UX issues, but I need to digg in deeper...

  44. laanwj commented at 8:55 AM on June 25, 2015: member

    @jonasschnelli Fixed the CSubNet::ToString function for subnets that can't be represented as /x, please pick the top commit from https://github.com/laanwj/bitcoin/tree/2015_06_cideriad

  45. jonasschnelli commented at 10:19 AM on June 25, 2015: contributor

    Added @laanwj commit on top to support non CIDR compatible netmasks.

  46. Diapolo commented at 7:25 AM on June 26, 2015: none

    @jonasschnelli I'm asking myself if this is worth adding the core signal BannedListChanged it makes it more complex and we don't need it. We pull the list / refresh the list via timer that calls void BanTableModel::refresh() anyway. Are you okay when I rework the signal handling to be easier?

  47. jonasschnelli commented at 7:31 AM on June 26, 2015: contributor

    @Diapolo: i extra added the signal to avoid a timer. For the peers list, a timer makes sense because we show recv/sent-bytes counters. The interval is 250ms (!). Poll updating through timers is a bad design IMO. Sometimes unavoidable.

    Adding another timer (or use the same one) for the ban list would mean unnecessarily using CPU ticks.

    Well placed signals/hocks allow a software to be extendable without creating unnecessary class coupling. I think we should not be to cautious we adding new signals.

  48. Diapolo commented at 7:45 AM on June 26, 2015: none

    Take a look at https://github.com/jonasschnelli/bitcoin/pull/5 IMHO this is not that bad and much easier flow-wise. We also don't have any core signals for the normal peer list. @laanwj What do you think?

    Perhaps a check could be added to verify we only pull a fresh list if the old one changed, that would reduce CPU ticks.

  49. jonasschnelli commented at 7:49 AM on June 26, 2015: contributor

    @Diapolo: i had a look at the PR and would prefer the current solution. Your solution would make generating a relatively static ban list every 250ms which need to go over a short locks of cs_setBanned.

    But don't have a strong opinion on that.

  50. Diapolo commented at 7:54 AM on June 26, 2015: none

    I'll try to rethink how this can be further improved, but code-wise it's much easier to understand. I respect your arguments for the core signals, but it feels like too much for this.

  51. jonasschnelli commented at 7:57 AM on June 26, 2015: contributor

    @Diapolo: can you point out the drawbacks of adding new signal to the main signals?

  52. Diapolo commented at 8:15 AM on June 26, 2015: none

    I had the impression the code-flow is overly complex, also because of updateBanlist in clientmodel, which you manually call from inside the rpcconsole. Also we have that timer code but it was disabled. The other way could be to remove the timer code and leave the core signal stuff, but also simplify clientmodel usage.

  53. jonasschnelli commented at 8:25 AM on June 26, 2015: contributor

    Good point. Added a commit that removes the unused timer code in bantablemodel.cpp | .h. The signaling approach over ClientModel is totally fine IMO. I would even say the complexity of the code is much higher with a timer than with a simple signal.

  54. Diapolo commented at 8:26 AM on June 26, 2015: none

    See https://github.com/jonasschnelli/bitcoin/pull/6, which should further simplify the code a little :). I rebased the pull to current master, this is just the new commit: https://github.com/Diapolo/bitcoin/commit/025954cb2fe1cfbbd14a5b0e86984197c5c7caa3

  55. Diapolo commented at 1:02 PM on June 26, 2015: none

    This is progressing nicely, hope we can get some consensus on my last 3 commits in https://github.com/jonasschnelli/bitcoin/pull/6 and then get this merged :).

  56. jonasschnelli commented at 1:13 PM on June 26, 2015: contributor

    @Diapolo: Please stop improving this PR within this PR. It's contra-productive. I can't follow all your change requests and it might crushes down this PR. Some of your things are really cool (like the sorting). But you mix everything together and that makes it hard to read and distinct between useful and not useful changes.

    Let stabilize this PR with searching and solving real problems. Once – and if – this is merge, you can PR your optimizations.

  57. Diapolo commented at 1:19 PM on June 26, 2015: none

    Too bad... as I'm solving "real" problems ;). I won't add anything new here I promise, but it would be cool to get the things I have opened in.

  58. jonasschnelli force-pushed on Jun 26, 2015
  59. jonasschnelli commented at 1:34 PM on June 26, 2015: contributor

    Added three cleanup/optimization PR from @Diapolo on top. Now it would be good to stabilize this PR.

  60. in src/qt/rpcconsole.cpp:None in 90b9409df3 outdated 
     322 | -        contextMenu = new QMenu();
 323 | -        contextMenu->addAction(disconnectAction);
 324 | -
 325 | -        // context menu signals
 326 | -        connect(ui->peerWidget, SIGNAL(customContextMenuRequested(const QPoint&)), this, SLOT(showMenu(const QPoint&)));
 327 | +        QAction* banAction1h      = new QAction(tr("&Ban Node for") + " " + tr("&1 hour"), this);
    Diapolo commented at 3:25 PM on June 26, 2015:

    I rechecked the code and during a translation for my build I came to this. The & is used as a shortcut when pressing ALT IMHO, so this will produce 2 shortcuts for every single menu entry. I'd suggest just removing them, because it will be hard to make them unique and still usable.

    Edit: I can't even access them on Windows... perhaps they are not wanted in context menus? What about Mac @jonasschnelli can you use these?

    Diapolo commented at 5:47 AM on July 3, 2015:

    Ping

  61. Diapolo commented at 5:47 AM on July 3, 2015: none

    Would be great if you can rebase this, as the banlist.dat change was merged :). Looking forward to now get this in.

  62. jonasschnelli force-pushed on Jul 3, 2015
  63. jonasschnelli force-pushed on Jul 3, 2015
  64. jonasschnelli force-pushed on Jul 3, 2015
  65. jonasschnelli commented at 6:43 AM on July 3, 2015: contributor

    Rebased and fix @Diapolos nits. Also made the ban table heading text non-selectable.

  66. jonasschnelli force-pushed on Jul 3, 2015
  67. jonasschnelli commented at 7:49 AM on July 3, 2015: contributor

    Added another commit from @Diapolo that stores the banlist.dat file after banning/unbanning over QT.

  68. Diapolo commented at 8:18 AM on July 3, 2015: none

    ACK

  69. jonasschnelli force-pushed on Jul 6, 2015
  70. jonasschnelli force-pushed on Jul 6, 2015
  71. Diapolo commented at 9:28 AM on July 29, 2015: none

    Can this be rebased and get some love :)? Would be great to get this in soon...

  72. jonasschnelli commented at 11:41 AM on July 29, 2015: contributor

    This PR doesn't need rebase and already has plenty love. Ready for merge IMO.

  73. jonasschnelli commented at 12:13 PM on July 29, 2015: contributor

    Here are some binaries if somebody want to test without ability to build: https://builds.jonasschnelli.ch/pulls/6315/

  74. jonasschnelli commented at 12:35 PM on July 29, 2015: contributor

    @Diapolo: you where right, needed adaption to the post QT_NO_KEYWORDS code. Rebased and fixed.

  75. jonasschnelli force-pushed on Jul 29, 2015
  76. jonasschnelli force-pushed on Aug 3, 2015
  77. jonasschnelli force-pushed on Aug 3, 2015
  78. jonasschnelli force-pushed on Aug 3, 2015
  79. jgarzik commented at 1:43 PM on September 16, 2015: contributor

    ut ACK - ready to merge once conflicts fixed via rebase

  80. jonasschnelli force-pushed on Sep 16, 2015
  81. jonasschnelli commented at 1:46 PM on September 16, 2015: contributor

    Rebased (trivial).

  82. Diapolo commented at 2:08 PM on September 16, 2015: none

    Happy to see this moving again, I had the impression our GUI is dead...

  83. [Qt] add ban functions to peers window
    add ban option for peer context menu (1h, 24h, 7d, 1y).
    50f090884c
  84. [Qt] add banlist table below peers table ad204df1a9
  85. [Qt] add ui signal for banlist changes 5f42132950
  86. [Qt] add context menu with unban option to ban table 770ca79aa0
  87. [Qt] banlist, UI optimizing and better signal handling 6135309816
  88. [Qt] bantable fix timestamp 64bit issue f0bcbc4c8a
  89. [Qt] bantable overhaul
    - some code cleanups
- fix date formatting
- reduce header includes
    53caec66cc
  90. net: use CIDR notation in CSubNet::ToString() 607809f037
  91. [Qt] polish ban table 9e521c1735
  92. net: Fix CIDR notation in ToString()
    Only use CIDR notation if the netmask can be represented as such.
    e2b8028e4c
  93. [Qt] remove unused timer-code from banlistmodel.cpp 43c1f5b8d7
  94. [Qt] simplify ban list signal handling
    - remove banListChanged signal from client model
- directly call clientModel->getBanTableModel()->refresh() without the way
  over clientModel->updateBanlist()

- also fix clearing peer detail window, when selecting (clicking)
  peers in the ban list
    cdd72cd5fb
  95. [Qt] bantable polish
    - add missing NULL pointer checks
- add better comments and reorder some code in rpcconsole.cpp
- remove unneeded leftovers in bantable.cpp
- update bantable column sizes to prevent cutting of banned until
    51654deff2
  96. [Qt] add sorting for bantable 65abe91ce4
  97. [Qt] adapt QT ban option to banlist.dat changes b1189cfa10
  98. [Qt] reenabling hotkeys for ban context menu, use different words
    - 1 (h)our
- 1 (d)ay
- 1 (w)eek
- 1 (y)ear
    be8929265f
  99. [Qt] call DumpBanlist() when baning unbaning nodes
    - this matches RPC call behaviour
    4ed05101f3
  100. [QA] fix netbase tests because of new CSubNet::ToString() output 07f70b2dde
  101. [QA] adabt QT_NO_KEYWORDS for QT ban implementation 7f90ea78cb
  102. jonasschnelli force-pushed on Sep 16, 2015
  103. jonasschnelli commented at 4:32 PM on September 16, 2015: contributor

    Rebased. Travis is happy now.

  104. jtimon commented at 5:00 PM on September 17, 2015: contributor

    Concept ACK

  105. luke-jr commented at 12:12 AM on September 20, 2015: member

    Why does 7f90ea78cb68c60408df85d5c653257dbc9160fe remove the DumpBanlist calls?

  106. [QT] dump banlist to disk in case of ban/unban over QT 7aac6db6eb
  107. jonasschnelli commented at 8:43 AM on September 20, 2015: contributor

    Thank @luke-jr for the precise review. I think this sneaked in over a rebase because this PR started before the banlist was persisted to disk.

    Just added a commit that re-implenents the DumpBanlist() calls in case of banning/unbanning over QT.

  108. luke-jr commented at 2:04 PM on September 20, 2015: member

    Note I did not do a precise review for this (yet?), just noticed those disappear from the previous version. ;)

  109. laanwj merged this on Sep 22, 2015
  110. laanwj closed this on Sep 22, 2015
  111. laanwj referenced this in commit e59d2a80f9 on Sep 22, 2015
  112. Diapolo commented at 12:01 PM on September 22, 2015: none

    @laanwj Now if #6371 could be merged, I'd be happy!

  113. zkbot referenced this in commit d0b67191e2 on Apr 5, 2018
  114. zkbot referenced this in commit bf0f1cbee9 on Mar 6, 2020
  115. zkbot referenced this in commit 2d9a9aaa83 on Nov 11, 2020
  116. zkbot referenced this in commit f40121446d on Nov 12, 2020
  117. zkbot referenced this in commit 049951dc45 on Feb 11, 2021
  118. zkbot referenced this in commit b3a6729944 on Feb 16, 2021
  119. zkbot referenced this in commit e85265fbd5 on Feb 17, 2021
  120. zkbot referenced this in commit b4b07a1bbd on Feb 17, 2021
  121. MarcoFalke locked this on Sep 8, 2021
Contributors
jonasschnelliDiapolopenguin1333luke-jrlaanwjjgarzikjtimon
Labels
GUI
Linked (view graph)
#6839 clang-format: Recently added files
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-21 18:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me