Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpc: add hidden getrawaddrman RPC to list addrman table entries #28523

Merged
merged 2 commits into from Oct 3, 2023

Conversation

0xB10C
Copy link
Contributor

@0xB10C 0xB10C commented Sep 23, 2023

Inspired by getaddrmaninfo (#27511), this adds a hidden/test-only getrawaddrman RPC. The RPC returns information on all addresses in the address manager new and tried tables. Addrman table contents can be used in tests and during development.

The RPC result encodes the bucket and position, the internal location of addresses in the tables, in the address object's string key. This allows users to choose to consume or to ignore the location information. If the internals of the address manager implementation change, the location encoding might change too.

getrawaddrman

EXPERIMENTAL warning: this call may be changed in future releases.

Returns information on all address manager entries for the new and tried tables.

Result:
{                                  (json object)
  "table" : {                      (json object) buckets with addresses in the address manager table ( new, tried )
    "bucket/position" : {          (json object) the location in the address manager table (<bucket>/<position>)
      "address" : "str",           (string) The address of the node
      "port" : n,                  (numeric) The port number of the node
      "network" : "str",           (string) The network (ipv4, ipv6, onion, i2p, cjdns) of the address
      "services" : n,              (numeric) The services offered by the node
      "time" : xxx,                (numeric) The UNIX epoch time when the node was last seen
      "source" : "str",            (string) The address that relayed the address to us
      "source_network" : "str"     (string) The network (ipv4, ipv6, onion, i2p, cjdns) of the source address
    },
    ...
  },
  ...
}

Examples:
> bitcoin-cli getrawaddrman
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "getrawaddrman", "params": []}' -H 'content-type: text/plain;' http://127.0.0.1:8332/

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 23, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK willcl-ark, amitiuttarwar, stratospher, achow101
Concept ACK ajtowns
Stale ACK brunoerg

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #28565 (rpc: getaddrmaninfo followups by stratospher)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@amitiuttarwar
Copy link
Contributor

concept ACK. I think it makes sense to expose more info about the addrman internals to allow for better observations & tooling

@brunoerg
Copy link
Contributor

Concept ACK

src/addrman.cpp Outdated Show resolved Hide resolved
src/rpc/net.cpp Outdated Show resolved Hide resolved
@0xB10C 0xB10C force-pushed the 2023-09-verbose-getaddrmaninfo branch from 48dfc81 to da44fd3 Compare September 25, 2023 12:17
@0xB10C
Copy link
Contributor Author

0xB10C commented Sep 25, 2023

I've also been hacking on a tool that visualizes addrman tables from the getaddrmaninfo verbose output in the browser. A wip version is on https://0xb10c.github.io/addrman-observer (github.com/0xB10C/addrman-observer). You can load dumps produced with bitcoin-cli getaddrmaninfo true > getaddrmaninfo.json (all processing happens locally).

src/rpc/net.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

light ACK da44fd3

I intend to do more tests soon. Just tested it on a recent clearnet node and worked as expected.
Outputs: https://gist.github.com/brunoerg/e3643337b4c16923ee0e0f7e66bec4d9

@0xB10C 0xB10C force-pushed the 2023-09-verbose-getaddrmaninfo branch 2 times, most recently from ede5ecf to 4307c8c Compare September 27, 2023 17:15
Copy link
Contributor

@ajtowns ajtowns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK

src/rpc/net.cpp Outdated Show resolved Hide resolved
src/rpc/net.cpp Outdated Show resolved Hide resolved
src/rpc/net.cpp Outdated Show resolved Hide resolved
src/rpc/net.cpp Outdated Show resolved Hide resolved
src/addrman.cpp Outdated Show resolved Hide resolved
@0xB10C
Copy link
Contributor Author

0xB10C commented Sep 29, 2023

Thanks for the helpful review comments @brunoerg @ajtowns @amitiuttarwar. I think I have addressed them all in the most recent push:

I've also updated OP and the tool on https://0xb10c.github.io/addrman-observer.

@0xB10C 0xB10C changed the title rpc: add getaddrmaninfo verbose to list addrman table entries rpc: add hidden getrawaddrman RPC to list addrman table entries Sep 29, 2023
@0xB10C 0xB10C force-pushed the 2023-09-verbose-getaddrmaninfo branch from 5c735ed to 45a84d1 Compare September 29, 2023 10:18
Copy link
Contributor

@stratospher stratospher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested ACK 45a84d1. Very cool to have!

src/addrman.cpp Outdated Show resolved Hide resolved
test/functional/rpc_net.py Outdated Show resolved Hide resolved
Copy link
Contributor

@amitiuttarwar amitiuttarwar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested ACK 45a84d1. All the review comments are small style/doc things. The functionality looks good.

src/addrman_impl.h Outdated Show resolved Hide resolved
src/rpc/net.cpp Outdated Show resolved Hide resolved
test/functional/rpc_net.py Outdated Show resolved Hide resolved
test/functional/rpc_net.py Outdated Show resolved Hide resolved
test/functional/rpc_net.py Show resolved Hide resolved
src/addrman.h Outdated Show resolved Hide resolved
src/addrman.h Show resolved Hide resolved
@0xB10C 0xB10C force-pushed the 2023-09-verbose-getaddrmaninfo branch from 45a84d1 to 584e12d Compare September 29, 2023 22:03
@0xB10C
Copy link
Contributor Author

0xB10C commented Sep 29, 2023

Addresses the comments and nits. Thanks @amitiuttarwar, @stratospher, and @ajtowns.

Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crACK 2dc1a6d

@DrahtBot DrahtBot requested review from stratospher and removed request for stratospher October 2, 2023 12:30
Exposing address manager table entries in a hidden RPC allows to introspect
addrman tables in tests and during development.

As response JSON object the following FORMAT1 is choosen:
{
  "table": {
    "<bucket>/<position>": { "address": "..", "port": .., ... },
    "<bucket>/<position>": { "address": "..", "port": .., ... },
    "<bucket>/<position>": { "address": "..", "port": .., ... },
    ...
  }
}

An alternative would be FORMAT2
{
  "table": {
    "bucket": {
      "position": { "address": "..", "port": .., ... },
      "position": { "address": "..", "port": .., ... },
      ..
    },
    "bucket": {
      "position": { "address": "..", "port": .., ... },
      ..
    },
  }
}

FORMAT1 and FORMAT2 have different encodings for the location of the
address in the address manager. While FORMAT2 might be easier to process
for downstream tools, it also mimics internal addrman mappings, which
might change at some point. Users not interested in the address location
can ignore the location key. They don't have to adapt to a new RPC
response format, when the internal addrman layout changes. Additionally,
FORMAT1 is also slightly easier to to iterate in downstream tools. The
RPC response-building implemenation complexcity is lower with FORMAT1
as we can more easily build a "<bucket>/<position>" key than a multiple
"bucket" objects with multiple "position" objects (FORMAT2).
@DrahtBot DrahtBot requested review from stratospher and removed request for stratospher October 2, 2023 13:42
Test that the getrawaddrman returns the addresses in the new and tried
tables. We can't check the buckets and positions as these are not
deterministic (yet).
@0xB10C 0xB10C force-pushed the 2023-09-verbose-getaddrmaninfo branch from 2dc1a6d to 352d5eb Compare October 2, 2023 13:48
@0xB10C
Copy link
Contributor Author

0xB10C commented Oct 2, 2023

With #28176 merged, I've rebased and now use the addrman table size constants in netutil.py in the test framework. This also resolves #28523 (comment). Sorry for invalidating the ACKs again.

Re-reviewers might want to use git range-diff 2dc1a6d...352d5eb.

@willcl-ark
Copy link
Member

reACK 352d5eb

git range-diff shows the only difference since previous ACK @ 2dc1a6d is using the just-merged constants from netutil.py:

Details
₿ git range-diff 2dc1a6d...352d5eb
 -:  ---------- >  1:  63e90e1d3f test: check for specific disconnect reasons in p2p_blockfilters.py
 -:  ---------- >  2:  2ab7952bda test: add bip157 coverage for (start height > stop height) disconnect
 -:  ---------- >  3:  5671c15f45 blockstorage: Mark FindBlockPos as nodiscard
 -:  ---------- >  4:  f0207e0030 blockstorage: Return on fatal block file flush error
 -:  ---------- >  5:  d8041d4e04 blockstorage: Return on fatal undo file flush error
 -:  ---------- >  6:  925bb723ca [refactor] batch-add transactions to DisconnectedBlockTransactions
 -:  ---------- >  7:  59a35a7398 [bench] DisconnectedBlockTransactions
 -:  ---------- >  8:  79ce9f0aa4 add std::list to memusage
 -:  ---------- >  9:  2765d6f343 rewrite DisconnectedBlockTransactions as a list + map
 -:  ---------- > 10:  cf5f1faa03 MOVEONLY: DisconnectedBlockTransactions to its own file
 -:  ---------- > 11:  4313c77400 make DisconnectedBlockTransactions responsible for its own memory management
 -:  ---------- > 12:  fa389d902f refactor: Drop unused fclose() from BufferedFile
 -:  ---------- > 13:  9999b89cd3 Make BufferedFile to be a CAutoFile wrapper
 -:  ---------- > 14:  fa56c421be Return CAutoFile from BlockManager::Open*File()
 -:  ---------- > 15:  fa4a9c0f43 Remove unused GetType() from OverrideStream, CVectorWriter, SpanReader
 -:  ---------- > 16:  fa72f09d6f Remove CHashWriter type
 -:  ---------- > 17:  fac29a0ab1 Remove SER_GETHASH, hard-code client version in CKeyPool serialize
 -:  ---------- > 18:  96b3f2dbe4 test: add unit test coverage for Python ECDSA implementation
 -:  ---------- > 19:  12f7257b8f doc: Be vague instead of wrong about MALLOC_ARENA_MAX
 -:  ---------- > 20:  b3db8c9d5c rpc: bumpfee, improve doc for 'reduce_output' arg
 -:  ---------- > 21:  4660fc82a1 wallet: Check last block and conflict height are valid in MarkConflicted
 -:  ---------- > 22:  782701ce7d test: Test loading wallets with conflicts without a chain
 -:  ---------- > 23:  b0f5175c04 net: Drop v2 garbage authentication packet
 -:  ---------- > 24:  e3720bca39 net: Simplify v2 recv logic by decoupling AAD from state machine
 -:  ---------- > 25:  fa40b3ee22 test: Avoid test failure on Linux root without cap-add LINUX_IMMUTABLE
 -:  ---------- > 26:  b5a962564e tests: Use manual bumps instead of bumpfee for resendwallettransactions
 -:  ---------- > 27:  d9841a7ac6 Add make_secure_unique helper
 -:  ---------- > 28:  6ef405ddb1 key: don't allocate secure mem for null (invalid) key
 -:  ---------- > 29:  262ab8ef78 Add package evaluation fuzzer
 -:  ---------- > 30:  f9047771d6 lint: fix custom mypy cache dir setting
 -:  ---------- > 31:  380130d9d7 test: add coverage to feature_addrman.py
 -:  ---------- > 32:  d9b172cd00 doc: fix link to developer-notes.md file in multiprocess.md
 1:  7303dac506 = 33:  da384a286b rpc: getrawaddrman for addrman entries
 2:  2dc1a6d59e ! 34:  352d5eb2a9 test: getrawaddrman RPC
    @@ Commit message
         deterministic (yet).

      ## test/functional/rpc_net.py ##
    -@@ test/functional/rpc_net.py: from test_framework.util import (
    - )
    - from test_framework.wallet import MiniWallet
    +@@ test/functional/rpc_net.py: from itertools import product
    + import time

    -+# Address manager size constants as defined in addrman_impl.h
    -+ADDRMAN_NEW_BUCKET_COUNT = 1 << 10
    -+ADDRMAN_TRIED_BUCKET_COUNT = 1 << 8
    -+ADDRMAN_BUCKET_SIZE = 1 << 6
    -+
    -
    - def assert_net_servicesnames(servicesflag, servicenames):
    -     """Utility that checks if all flags are correctly decoded in
    + import test_framework.messages
    ++from test_framework.netutil import ADDRMAN_NEW_BUCKET_COUNT, ADDRMAN_TRIED_BUCKET_COUNT, ADDRMAN_BUCKET_SIZE
    + from test_framework.p2p import (
    +     P2PInterface,
    +     P2P_SERVICES,
     @@ test/functional/rpc_net.py: class NetTest(BitcoinTestFramework):
              self.test_addpeeraddress()
              self.test_sendmsgtopeer()

@DrahtBot DrahtBot requested review from stratospher and brunoerg and removed request for stratospher October 2, 2023 13:58
@amitiuttarwar
Copy link
Contributor

reACK 352d5eb

@DrahtBot DrahtBot requested review from stratospher and removed request for amitiuttarwar and stratospher October 2, 2023 18:30
@stratospher
Copy link
Contributor

reACK 352d5eb.

@DrahtBot DrahtBot removed the request for review from stratospher October 3, 2023 03:24
@achow101
Copy link
Member

achow101 commented Oct 3, 2023

ACK 352d5eb

@achow101 achow101 merged commit 01bd9d7 into bitcoin:master Oct 3, 2023
16 checks passed
/*multiplicity_in=*/from_tried ? 1 : info.nRefCount,
bucket,
position);
infos.push_back(std::make_pair(info, location));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::make_pair isn't needed, starting with C++11, you can use emplace_back. Fixed in #28583

assert_equal(result["source"], expected["source"])
assert_equal(result["source_network"], expected["source_network"])
# To avoid failing on slow test runners, use a 10s vspan here.
assert_approx(result["time"], time.time(), vspan=10)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

 test  2023-10-12T14:27:36.383000Z TestFramework (DEBUG): Test that the getrawaddrman contains information about the addresses added in a previous test 
 node1 2023-10-12T14:27:36.384588Z [http] [httpserver.cpp:307] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:37962 
 node1 2023-10-12T14:27:36.385003Z [httpworker.1] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=getrawaddrman user=__cookie__ 
 node1 2023-10-12T14:27:36.385345Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:36.385576Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.17ms) 
 node1 2023-10-12T14:27:36.988817Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:36.989107Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.17ms) 
 node1 2023-10-12T14:27:36.989261Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:36.989419Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 node1 2023-10-12T14:27:37.083728Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.094364Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (10.42ms) 
 node1 2023-10-12T14:27:37.096430Z [http] [httpserver.cpp:307] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:37962 
 node1 2023-10-12T14:27:37.096690Z [httpworker.1] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=getaddrmaninfo user=__cookie__ 
 node1 2023-10-12T14:27:37.096908Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.097184Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.21ms) 
 node1 2023-10-12T14:27:37.097247Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.097385Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.10ms) 
 node1 2023-10-12T14:27:37.097434Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.097563Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.10ms) 
 node1 2023-10-12T14:27:37.097606Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.097715Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.08ms) 
 node1 2023-10-12T14:27:37.097757Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.097854Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.07ms) 
 node1 2023-10-12T14:27:37.097883Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.175048Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (74.78ms) 
 node1 2023-10-12T14:27:37.179840Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.180187Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.27ms) 
 node1 2023-10-12T14:27:37.180243Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.180383Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 node1 2023-10-12T14:27:37.180466Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.180610Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 node1 2023-10-12T14:27:37.180652Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.180790Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.10ms) 
 node1 2023-10-12T14:27:37.180850Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.180980Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.09ms) 
 node1 2023-10-12T14:27:37.181023Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.181160Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.10ms) 
 node1 2023-10-12T14:27:37.181234Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.181363Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.09ms) 
 node1 2023-10-12T14:27:37.181406Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.181545Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.10ms) 
 node1 2023-10-12T14:27:37.181605Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.181738Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.10ms) 
 node1 2023-10-12T14:27:37.181780Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.181948Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.10ms) 
 node1 2023-10-12T14:27:37.182008Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.182146Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.10ms) 
 node1 2023-10-12T14:27:37.182187Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.182321Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.10ms) 
 node1 2023-10-12T14:27:37.182393Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.182545Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 node1 2023-10-12T14:27:37.182588Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.182724Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.10ms) 
 node1 2023-10-12T14:27:37.182786Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.182926Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.10ms) 
 node1 2023-10-12T14:27:37.182975Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.183118Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 node1 2023-10-12T14:27:37.183181Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.183325Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.10ms) 
 node1 2023-10-12T14:27:37.183393Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.183544Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 node1 2023-10-12T14:27:37.183617Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.183772Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.12ms) 
 node1 2023-10-12T14:27:37.183812Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.183955Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 node1 2023-10-12T14:27:37.184068Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.184220Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.12ms) 
 node1 2023-10-12T14:27:37.184261Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.184404Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 node1 2023-10-12T14:27:37.184460Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.184607Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 node1 2023-10-12T14:27:37.184646Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.184790Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 node1 2023-10-12T14:27:37.184868Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.185043Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.14ms) 
 node1 2023-10-12T14:27:37.185086Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.185255Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.13ms) 
 node1 2023-10-12T14:27:37.185313Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.185483Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 node1 2023-10-12T14:27:37.185524Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.185671Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 node1 2023-10-12T14:27:37.185734Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.185883Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.12ms) 
 node1 2023-10-12T14:27:37.185922Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: new 1, tried 1, total 2 started 
 node1 2023-10-12T14:27:37.186069Z [httpworker.1] [logging/timer.h:58] [Log] [addrman] CheckAddrman: completed (0.11ms) 
 test  2023-10-12T14:27:37.273000Z TestFramework (ERROR): Assertion failed 
                                   Traceback (most recent call last):
                                     File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main
                                       self.run_test()
                                     File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/rpc_net.py", line 71, in run_test
                                       self.test_getrawaddrman()
                                     File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/rpc_net.py", line 465, in test_getrawaddrman
                                       check_getrawaddrman_entries(expected)
                                     File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/rpc_net.py", line 432, in check_getrawaddrman_entries
                                       check_addr_information(entry, expected_entry)
                                     File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/rpc_net.py", line 411, in check_addr_information
                                       assert_approx(result["time"], time.time(), vspan=10)
                                     File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/util.py", line 38, in assert_approx
                                       raise AssertionError("%s < [%s..%s]" % (str(v), str(vexp - vspan), str(vexp + vspan)))
                                   AssertionError: 1697120845 < [1697120847.2721245..1697120867.2721245]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's a reasonable time range for slow test runners?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the data is added in another test case, so I don't think an upper time can be specified, considering that it is possible that someone writes more test cases in the future and adds them in between the two test cases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this check should either be removed, or made an exact check (via mocktime) or some other way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref #28671

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref #28671

@0xB10C 0xB10C deleted the 2023-09-verbose-getaddrmaninfo branch October 13, 2023 11:10
Frank-GER pushed a commit to syscoin/syscoin that referenced this pull request Oct 13, 2023
…n table entries

352d5eb test: getrawaddrman RPC (0xb10c)
da384a2 rpc: getrawaddrman for addrman entries (0xb10c)

Pull request description:

  Inspired by `getaddrmaninfo` (bitcoin#27511), this adds a hidden/test-only `getrawaddrman` RPC. The RPC returns information on all addresses in the address manager new and tried tables. Addrman table contents can be used in tests and during development.

  The RPC result encodes the `bucket` and `position`, the internal location of addresses in the tables, in the address object's string key. This allows users to choose to consume or to ignore the location information. If the internals of the address manager implementation change, the location encoding might change too.

  ```
  getrawaddrman

  EXPERIMENTAL warning: this call may be changed in future releases.

  Returns information on all address manager entries for the new and tried tables.

  Result:
  {                                  (json object)
    "table" : {                      (json object) buckets with addresses in the address manager table ( new, tried )
      "bucket/position" : {          (json object) the location in the address manager table (<bucket>/<position>)
        "address" : "str",           (string) The address of the node
        "port" : n,                  (numeric) The port number of the node
        "network" : "str",           (string) The network (ipv4, ipv6, onion, i2p, cjdns) of the address
        "services" : n,              (numeric) The services offered by the node
        "time" : xxx,                (numeric) The UNIX epoch time when the node was last seen
        "source" : "str",            (string) The address that relayed the address to us
        "source_network" : "str"     (string) The network (ipv4, ipv6, onion, i2p, cjdns) of the source address
      },
      ...
    },
    ...
  }

  Examples:
  > bitcoin-cli getrawaddrman
  > curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "getrawaddrman", "params": []}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
  ```

ACKs for top commit:
  willcl-ark:
    reACK 352d5eb
  amitiuttarwar:
    reACK 352d5eb
  stratospher:
    reACK 352d5eb.
  achow101:
    ACK 352d5eb

Tree-SHA512: cc462666b5c709617c66b0e3e9a17c4c81e9e295f91bdd9572492d1cb6466fc9b6d48ee805ebe82f9f16010798370effe5c8f4db15065b8c7c0d8637675d615e
fanquake added a commit to bitcoin-core/gui that referenced this pull request Oct 19, 2023
fa4c683 test: Fix failing time check in rpc_net.py (MarcoFalke)

Pull request description:

  This check fails on slow runners, such as s390x qemu.

  Fix it by using mocktime.

  See bitcoin/bitcoin#28523 (comment)

ACKs for top commit:
  0xB10C:
    ACK fa4c683
  pinheadmz:
    ACK fa4c683
  brunoerg:
    crACK fa4c683

Tree-SHA512: 83fb534682e11e97537dc89de8c16f206f38af1a892a2d5970c02684c05eaea8fc9adba3159f16b2440ca0b3871d513a0562a6f3a38f19a5574a47be0919e42f
Frank-GER pushed a commit to syscoin/syscoin that referenced this pull request Oct 21, 2023
fa4c683 test: Fix failing time check in rpc_net.py (MarcoFalke)

Pull request description:

  This check fails on slow runners, such as s390x qemu.

  Fix it by using mocktime.

  See bitcoin#28523 (comment)

ACKs for top commit:
  0xB10C:
    ACK fa4c683
  pinheadmz:
    ACK fa4c683
  brunoerg:
    crACK fa4c683

Tree-SHA512: 83fb534682e11e97537dc89de8c16f206f38af1a892a2d5970c02684c05eaea8fc9adba3159f16b2440ca0b3871d513a0562a6f3a38f19a5574a47be0919e42f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants