-
Notifications
You must be signed in to change notification settings - Fork 46
Add hidden RPC getrawaddrman
#439
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
base: master
Are you sure you want to change the base?
Conversation
|
Ok so I learned that only the public RPCs should be added to the rustdoc table in the |
bd26946 to
bafede8
Compare
| .find(|e| e.address == peer_address && e.port == peer_port) | ||
| .expect("added peer should appear in the 'new' table"); | ||
|
|
||
| assert_eq!(entry.network, "ipv4"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| assert_eq!(entry.network, "ipv4"); | |
| assert_eq!(entry.network, "ipv4"); | |
| assert!(entry.services > 0); | |
| assert!(entry.time > 0); |
I don't recall what source is set to when using addpeeraddress, but maybe that can be checked as well.
integration_test/tests/hidden.rs
Outdated
| assert_eq!(entry.network, "ipv4"); | ||
|
|
||
| // mapped_as field added in v28, only present with -asmap config. | ||
| assert!(entry.mapped_as.is_none(), "mapped_as requires -asmap config"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| assert!(entry.mapped_as.is_none(), "mapped_as requires -asmap config"); | |
| assert!(entry.mapped_as.is_none(), "mapped_as requires -asmap config"); | |
| assert!(entry.source_mapped_as.is_none(), "mapped_as requires -asmap config"); |
This might break once/if asmap get's included and enabled by default. But that's probably a few versions out.
|
Looks good to me! https://github.com/0xB10C/addrman-observer still uses a custom rust-bitcoincore-rpc |
I was going to propagate the change once all good here, but go for it! |
types/src/v26/hidden.rs
Outdated
| /// Mapped AS (Autonomous System) number at the end of the BGP route to the peer. | ||
| /// Only present if the -asmap config option is set. | ||
| /// Added in Bitcoin Core v28. | ||
| pub mapped_as: Option<u32>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this and props for getting as far as you did in this convoluted repo. The version specific types are designed to be exactly correct for the version i.e., v26::RawAddrManEntry should be exactly the shape returned by Core v26. This means no optional fields. Rather anytime a new field is added a new version of the struct is added, so we should have a v28::RawAddrMan (and v28::RawAddrManEntry).
(If there was an associated struct in model, where there is not in this case, it would use optional fields because the model types are version in-specific.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FTR looks like you have the client stuff correct, because the v28 changes do not effect the client so the v26 client macro works just fine. I mentioned that this repo was convoluted, right?
|
Everything else looks good. Reviewed: bafede8 |
|
Heads up, we are going to cut a release shortly. Do you want to get this in it? |
Yeah, I'll prioritise (school holidays 😬) |
|
No stress, holla if you need a hand. |
Add support for the hidden getrawaddrman RPC (v26) that returns raw address manager table entries. This RPC is marked as experimental and hidden from the API docs by default. The mapped_as and source_mapped_as fields are Optional<u32> following the established codebase pattern for version-specific fields. These fields were added in Bitcoin Core v28 and will deserialize as None for earlier versions or when -asmap is not configured. Includes integration test.
bafede8 to
a7a8785
Compare
|
The |
Add support for the hidden
getrawaddrmanRPC that returns raw address manager table entries (added v26). This RPC is marked as experimental and hidden from the API docs by default.The
mapped_asandsource_mapped_asfields areOption<u32>following the established pattern for version-specific fields (nointo.rsrequired because these are additions rather than removals or changed types). These fields were added in Bitcoin Core v28 and will deserialise asNonefor earlier versions or when-asmapis not configured.Note: will have conflicts with #435 and #433, happy to rebase after either merges