gui: display mapped AS in peers info window #18402

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.

fanquake added the label GUI on Mar 22, 2020

hebasto commented at 2:53 PM on March 22, 2020: member

Concept ACK. Smth similar was in my task list ;)

laanwj commented at 5:53 PM on March 22, 2020: member

A good addition! ACK 8580db114ae30459af245f2bf0bfcc5f8c85b612 (but see nits)

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

                 <property name="text">
                  <string>N/A</string>
                 </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.

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.

promag commented at 7:31 PM on March 22, 2020: member

Concept ACK, honestly you could squash these commits.

jonatack force-pushed on Mar 23, 2020

jonatack commented at 1:38 PM on March 23, 2020: member

Thanks @hebasto, @laanwj, and @promag for reviewing. Feedback taken; done.

hebasto approved

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.

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:

    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

promag commented at 5:39 PM on March 23, 2020: member

Code review ACK modulo suggestion.

gui: display Mapped AS in peers info window aae26053f9

gui: avoid QT Designer/Form Editor re-formatting

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>

76db4b260e

jonatack force-pushed on Mar 24, 2020

jonatack commented at 1:09 PM on March 24, 2020: member

Thanks @promag and @hebasto, done.

laanwj commented at 2:10 PM on March 25, 2020: member

ACK 76db4b260e4826a1e59a5e44c92e4c10ec986527

laanwj merged this on Mar 25, 2020

laanwj closed this on Mar 25, 2020

jonatack deleted the branch on Mar 25, 2020

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?

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.

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?

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.

jonasschnelli commented at 7:47 AM on August 14, 2020: contributor

Core Review ACK 76db4b260e4826a1e59a5e44c92e4c10ec986527

deadalnix referenced this in commit afdf7c7a75 on Jan 12, 2021

DrahtBot locked this on Feb 15, 2022