search

talaia-labs / rust-teos

The Eye of Satoshi - Lightning Watchtower
https://talaia-labs.github.io/talaia.watch/
MIT License
134 stars 63 forks source link

Adds addresses field to `gettowerinfo` #151

Closed sr-gi closed 1 year ago

sr-gi commented 1 year ago

This PR adds a new field to the gettowerinfo RPC call. The field includes data regarding the interfaces where the public API is being offered (i.e. ipv4 and/or tor).

The rationale for this is having the API endpoint information handy without having to check the logs (this is currently most relevant for the onion address).

Fixes https://github.com/talaia-labs/rust-teos/issues/143

cc/ @jochemin

sr-gi commented 1 year ago

Notice this is a breaking change for the protos, given it modifies them without updating its version. I don't think it is worth a version bump for them at the moment, maybe when more functionality has been added (or after the next major update).

sr-gi commented 1 year ago

Something I'd like to discuss here would be how to address proto updates without them being breaking changes. I haven't really paid much attention to this so far, and maybe we don't want to, but there are ways of making it backward compatible.

https://developers.google.com/protocol-buffers/docs/proto3#updating

sr-gi commented 1 year ago

Notice this is a breaking change for the protos, given it modifies them without updating its version. I don't think it is worth a version bump for them at the moment, maybe when more functionality has been added (or after the next major update).

Something I'd like to discuss here would be how to address proto updates without them being breaking changes. I haven't really paid much attention to this so far, and maybe we don't want to, but there are ways of making it backward compatible.

https://developers.google.com/protocol-buffers/docs/proto3#updating

I've rebased this and made the change non-breaking. This was breaking due to the field order, and the only good reason for the ordering was having addresses close to tower_id when serializing in json to show the result to the user. I don't think that actually reasonable, if we want a different ordering for the displayed result we can always implement our own serialization.

I've also moved the AddressType logic to common so it can be reused by the clients (e.g. to flag a given address as clearnet or tor). This is relevant to address the Tor related issues from #102 in a more elegant way.

TL;DR: this is non-breaking now. AddressType is now part of teos-common.

sr-gi commented 1 year ago

Should be good to go now