Add model for getnodeaddresses #191
Conversation
GetNodeAddresses returns an Address. Create a bitcoin::Address model for it and modify the test to use it. Implement for v18 to v21. In v22 the return changes, label v22 to v28 as TODO.
jamillambert
left a comment
jamillambert
left a comment
There was a problem hiding this comment.
The CI fail looks unrelated
|
Yeah we have intermitent CI failures atm, in #147 it was suggested its to do with flaky downloads of Core. |
I've had them locally too running the tests. |
|
Do you download locally or set |
|
Download locally. I'll try and dig a bit and see if I can find out more. But always hard with intermittent errors. |
|
See the alias' in |
I would guess we should just be adding repeat/timeout logic to the download code (in build.rs). |
|
Ouch, in review I did not notice that the address returned is not a Bitcoin address but a 'node' address. I don't actually know what that means, is it IP address? Eitherway the EDIT: In |
| if let Some(addr) = json.0.first() { | ||
| let res: Result<mtype::GetNodeAddresses, _> = json.into_model(); | ||
| let model = res.expect("GetNodeAddresses into model"); | ||
| assert!(model.0.len() <= 2500); |
There was a problem hiding this comment.
Nothing the list is empty 😂. It was an AI suggested sanity check that the list was smaller than the max allowed size.
| if let Some(addr) = model.0.first() { | ||
| assert!(addr.port > 0); | ||
| assert!(!addr.address.is_empty()); | ||
| assert!(addr.time > 1231006505); | ||
| } |
There was a problem hiding this comment.
This code block will never be run because there is only one node started. We'd need to start a multi-node network to test this.
There was a problem hiding this comment.
I opened #320 to follow this up. Adding a multi-node network did not work for me either.
054862f212ac5e8c7911fae7a0b64f565e3f9b7b Add model for getnodeaddresses (Jamil Lambert, PhD)
Pull request description:
GetNodeAddresses returns an Address. Create a bitcoin::Address model for it and modify the test to use it.
Implement for v18 to v21. In v22 the return changes, label v22 to v28 as TODO.
ACKs for top commit:
tcharding:
ACK 054862f212ac5e8c7911fae7a0b64f565e3f9b7b
Tree-SHA512: 771e760ef8330748c93d7e20731a8e46a6d51e4c1b4b9087e6ac38f07477d8441c041b053dd14803879e40414ed5bdc1ede2512b69ac7d486f15dd039d3c0923
2 participants
GetNodeAddresses returns an Address. Create a bitcoin::Address model for it and modify the test to use it.
Implement for v18 to v21. In v22 the return changes, label v22 to v28 as TODO.