create Address message with SocketAddr, get SocketAddr from Address m… #64
Conversation
|
cc @TheBlueMatt can you review this? If not I can do it this week. |
| #[test] | ||
| fn test_socket_addr () { | ||
| let s4 = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(111,222,123,4)), 5555); | ||
| let a4 = Address::new(&s4, 9); |
There was a problem hiding this comment.
Can you add a test that this mapps correctly to bitcoin's ipv6-mapped addresses? I assume its correct, but good to check as IIRC there are multiple ways to map (I belive Bitcoin uses the ::ffff:IPv4 one, but should double-check).
There was a problem hiding this comment.
That is what Ipv6Addr::to_ipv4 checks and I used in Address::socket_addr() therefore the test should never fail.
There was a problem hiding this comment.
Sure, I'm just saying cause I thought somehow there were multiple ways to wrap IPv4 in IPv6 (though it appears ::ffff:... is now standard), so wanted a check that it actually is using the one we expected. Does'nt matter much, though.
There was a problem hiding this comment.
see in deserialize_address_test
src/network/address.rs
Outdated
| } | ||
|
|
||
| /// extract socket address from an address message | ||
| pub fn socket_addr (&self) -> SocketAddr { |
There was a problem hiding this comment.
You should have a check that the contained addr isnt a Tor-wrapped one and return a Result<>, sadly, I think.
There was a problem hiding this comment.
Please give me a pointer how that is recognized. I did not yet deal with tor at this level.
There was a problem hiding this comment.
Its checked here in Bitcoin Core: https://github.com/bitcoin/bitcoin/blob/master/src/netaddress.cpp#L176
c12b315 to
e454a51
Compare
|
addressed comments and squashed commits |
d6cc04b to
9819bfb
Compare
|
@apoelstra please remove rust 1.14 check if there is no good reason for it. |
|
The Rust 1.14 check is what's required to support Debian stable, which a lot of folks use.
…On March 23, 2018 1:50:11 PM UTC, "Tamás Blummer" ***@***.***> wrote:
@apoelstra please remove rust 1.14 check if there is no good reason for
it.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#64 (comment)
|
6d82a63 to
ae70844
Compare
|
updated for rust 1.14 squashed. |
Fix some optionals
…ing them (rust-bitcoin#64) * quorum sigs from message * quorum sigs from message * quorum sigs from message * more work * more work * more work * more work * more work * work to get this working * change * change * clean up * fixes * bls8 * fmt * bump to version 39
…essage.
Creating addr message for a socket or socket from an addr message is a common task upstream. It makes sense to support in this base library.