gui: display mapped AS in peers info window #18402

pull jonatack wants to merge 2 commits into bitcoin:master from jonatack:gui-peers-display-mapped_as changing 2 files +46 −19
  1. jonatack commented at 12:08 pm on March 22, 2020: member

    Continuing the asmap integration of #16702 which added mapped_as to the rpc getpeerinfo output, this adds the mapped AS to the Peers detail window in the GUI wallet.

    $ src/qt/bitcoin-qt -asmap=<path-to-asmap-file> (asmap on)

    Screenshot from 2020-03-22 12-29-56

    $ src/qt/bitcoin-qt (asmap off)

    Screenshot from 2020-03-22 12-32-46

    Added a tooltip and a couple of minor fixups.

  2. fanquake added the label GUI on Mar 22, 2020
  3. hebasto commented at 2:53 pm on March 22, 2020: member
    Concept ACK. Smth similar was in my task list ;)
  4. laanwj commented at 5:53 pm on March 22, 2020: member
    A good addition! ACK 8580db114ae30459af245f2bf0bfcc5f8c85b612 (but see nits)
  5. in src/qt/rpcconsole.cpp:1124 in 8580db114a outdated
    1118@@ -1118,6 +1119,9 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats)
    1119     ui->peerDirection->setText(stats->nodeStats.fInbound ? tr("Inbound") : tr("Outbound"));
    1120     ui->peerHeight->setText(QString("%1").arg(QString::number(stats->nodeStats.nStartingHeight)));
    1121     ui->peerWhitelisted->setText(stats->nodeStats.m_legacyWhitelisted ? tr("Yes") : tr("No"));
    1122+    if (stats->nodeStats.m_mapped_as != 0) {
    1123+        ui->peerMappedAS->setText(QString("%1").arg(QString::number(stats->nodeStats.m_mapped_as)));
    1124+    }
    


    laanwj commented at 5:54 pm on March 22, 2020:
    I think you need to set something here in the case of 0 as well (otherwise, old info will stick around).

    jonatack commented at 6:29 pm on March 22, 2020:

    Thanks. It looks like in the case of 0 it already falls back to the “N/A” value in debugwindow.ui::1521

    0                 <property name="text">
    1                  <string>N/A</string>
    2                 </property>
    

    promag commented at 7:30 pm on March 22, 2020:
    Yes, you have to reset to default N/A label.

    jonatack commented at 1:37 pm on March 23, 2020:
    Deferring to your experience here. Done.
  6. in src/qt/forms/debugwindow.ui:1520 in 8580db114a outdated
    1515+               <item row="18" column="1">
    1516+                <widget class="QLabel" name="peerMappedAS">
    1517+                 <property name="cursor">
    1518+                  <cursorShape>IBeamCursor</cursorShape>
    1519+                 </property>
    1520+                 <property name="text">
    


    laanwj commented at 5:55 pm on March 22, 2020:
    notr="true"? Edit: no, other occurences of N/A are also translated.
  7. promag commented at 7:31 pm on March 22, 2020: member
    Concept ACK, honestly you could squash these commits.
  8. jonatack force-pushed on Mar 23, 2020
  9. jonatack commented at 1:38 pm on March 23, 2020: member
    Thanks @hebasto, @laanwj, and @promag for reviewing. Feedback taken; done.
  10. hebasto approved
  11. hebasto commented at 5:23 pm on March 23, 2020: member

    ACK 64f383857d6f3412e432e737c298ff17f84c48d3, tested on Linux Mint 19.3:

    DeepinScreenshot_bitcoin-qt_20200323191834


    @jonatack While debugwindow.ui is touched mind adding a commit to fix annoying re-formatting by Qt Designer? @laanwj #15220 (comment):

    please just do this the next time you’re editing these files.

  12. in src/qt/rpcconsole.cpp:1121 in 64f383857d outdated
    1117@@ -1118,6 +1118,7 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats)
    1118     ui->peerDirection->setText(stats->nodeStats.fInbound ? tr("Inbound") : tr("Outbound"));
    1119     ui->peerHeight->setText(QString("%1").arg(QString::number(stats->nodeStats.nStartingHeight)));
    1120     ui->peerWhitelisted->setText(stats->nodeStats.m_legacyWhitelisted ? tr("Yes") : tr("No"));
    1121+    ui->peerMappedAS->setText(stats->nodeStats.m_mapped_as == 0 ? tr("N/A") : QString("%1").arg(QString::number(stats->nodeStats.m_mapped_as)));
    


    promag commented at 5:39 pm on March 23, 2020:
    0    ui->peerMappedAS->setText(stats->nodeStats.m_mapped_as == 0 ? tr("N/A") : QString::number(stats->nodeStats.m_mapped_as));
    

    jonatack commented at 1:08 pm on March 24, 2020:
    @promag tested, LGTM, done and extended to peerVersion and peerHeight
  13. promag commented at 5:39 pm on March 23, 2020: member
    Code review ACK modulo suggestion.
  14. gui: display Mapped AS in peers info window aae26053f9
  15. gui: avoid QT Designer/Form Editor re-formatting
    Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
    76db4b260e
  16. jonatack force-pushed on Mar 24, 2020
  17. jonatack commented at 1:09 pm on March 24, 2020: member
    Thanks @promag and @hebasto, done.
  18. laanwj commented at 2:10 pm on March 25, 2020: member
    ACK 76db4b260e4826a1e59a5e44c92e4c10ec986527
  19. laanwj merged this on Mar 25, 2020
  20. laanwj closed this on Mar 25, 2020

  21. jonatack deleted the branch on Mar 25, 2020
  22. andronoob commented at 7:34 pm on August 13, 2020: none
    I wonder if the string “Mapped AS” displayed in the GUI could be worded in other ways? Like “BGP AS number”, so that it could be more self-explanatory, or at least more google-able?
  23. jonatack commented at 8:03 pm on August 13, 2020: member
    @andronoob there is an explanatory tooltip – “The mapped Autonomous System used for diversifying peer selection.” – see https://github.com/bitcoin/bitcoin/pull/18402/files#diff-2537e9bcbac21d35d8fa28ce1a35e89dR1508. In RPC getpeerinfo it is also named mapped_as. I think your translation is also inaccurate. See https://bitcoincore.reviews/16702 for more.
  24. andronoob commented at 6:54 am on August 14, 2020: none

    The mapped Autonomous System used for diversifying peer selection.

    To my understanding, such phrase means:

    “(Such number is) The Autonomous System (AS) (number, aka ASN) which this peer is mapped to (according to the map data). BGP AS is considered during peer selection, in order to diversify connected peers. (thus some risks related might be mitigated)”

    Correct me if I’m wrong?

  25. in src/qt/forms/debugwindow.ui:655 in 76db4b260e
    651@@ -652,12 +652,12 @@
    652          </item>
    653          <item>
    654           <widget class="QLineEdit" name="lineEdit">
    655-           <property name="placeholderText">
    


    jonasschnelli commented at 7:47 am on August 14, 2020:
    I guess the property order change has been done through QTCreator? I would have probably unstaged those.
  26. jonasschnelli commented at 7:47 am on August 14, 2020: contributor
    Core Review ACK 76db4b260e4826a1e59a5e44c92e4c10ec986527
  27. deadalnix referenced this in commit afdf7c7a75 on Jan 12, 2021
  28. DrahtBot locked this on Feb 15, 2022

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: 2024-11-17 18:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me