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

rfc: p2p light client #7672

Merged
merged 17 commits into from
Jan 31, 2022
Merged

Conversation

tychoish
Copy link
Contributor

No description provided.

Copy link
Contributor

@creachadair creachadair left a comment

Choose a reason for hiding this comment

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

I think this is a very good synopsis of the design space. Assuming consensus, is the idea that we'd do this as part of the libp2p migration? (Or at least part-concurrently with it)

docs/rfc/rfc-010-p2p-light-client.rst Outdated Show resolved Hide resolved
implementation of the reactor would likely be very straight forward, and the
implementation of the provider is similarly relatively simple.

The complexity of the project focuses around peer discovery, handling when
Copy link
Contributor

Choose a reason for hiding this comment

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

Would the peer footprint of a light client be (substantially) different from the peer footprint of the node qua node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to interpret "peer footprint" as "network traffic + work of servicing request" and I think the answer to this is yes: you still need the router processes, and the reactor process but I think the workload will be pretty small.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, by "peer footprint" I meant mainly the number and identity of peers, and the amount of data needed to communicate with them. It seems as though a p2p light client would not want to talk to a radically different set/volume of peers than a regular node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I presume that most of a light client would want to talk to 3 nodes at a time, potentially more here and there as nodes go off line and return, but in general, should be pretty small.

I worry more about the impact of light clients on full nodes.

LibP2P
~~~~~~

While some aspects of the P2P light client implementation are orthogonal to
Copy link
Contributor

Choose a reason for hiding this comment

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

This may also be a good testbed for the migration, since it is a use that doesn't currently exist in exactly this form.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree though I'm not sure how I would operationalize this or explore this. The libp2p migration is (to my mind) pretty far below the level of reactors (e.g. mostly just ripping out the bottom half or 2/3rds of the router.)

docs/rfc/rfc-010-p2p-light-client.rst Outdated Show resolved Hide resolved
docs/rfc/rfc-010-p2p-light-client.rst Outdated Show resolved Hide resolved
docs/rfc/rfc-010-p2p-light-client.rst Outdated Show resolved Hide resolved
docs/rfc/rfc-010-p2p-light-client.rst Outdated Show resolved Hide resolved

Given that the primary relayer implementation, is hermes (rust,) it might be
safe to deliver a version of tendermint that adds a light client rector in
the full nodes, but that does not provide an implementation of a Go light
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a really interesting possibility!

Copy link
Contributor

Choose a reason for hiding this comment

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

This kind of already exists. All nodes run state sync which means they already serve light blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, though the statesync plumbing for lightblocks isn't reusable outside of statesync, as I understand it.

Copy link
Contributor

Choose a reason for hiding this comment

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

The case you describe above, where the rust light client uses mconnection and just operates over the light block channel is possible. The problem of course is that the light client may get requests from other nodes but it can always just say I don't have them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it should be possible to expose to the light client reactor enough peer information (e.g. "is this node a data bearing node") to prevent these requests from being made in most cases (e.g. these light nodes will include the router and peer manager, and possibly PEX anyway, so we could find a way to prevent these requests in the pathological case.

Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

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

Nice write up! I left a few comments. I guess to me that a decision that can come from this RFC is that it doesn't make sense to have an implementation before LibP2P and that much of the discussion is around how will we use libp2p to match our requirements of a light client.

docs/rfc/rfc-010-p2p-light-client.rst Outdated Show resolved Hide resolved
docs/rfc/rfc-010-p2p-light-client.rst Outdated Show resolved Hide resolved
docs/rfc/rfc-010-p2p-light-client.rst Outdated Show resolved Hide resolved
docs/rfc/rfc-010-p2p-light-client.rst Outdated Show resolved Hide resolved
Comment on lines 82 to 85
also assumes that there are *no* other uses of the RPC in the relayer, and
unless the relayers have the option of dropping all RPC use, it's unclear if a
P2P light client will actually be able to successfully remove the dependency
on the RPC system.
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah so relayers still need to be able to query the application state (specifically the IBC module) for pending txs to transfer across. I know that the SDK are trying to migrate more to gRPC and thus it's possible that the only use of the RPC may be the "local" server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not quite sure I fully understand the implication of this.

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be that in a years time, no "external" client uses the Tendermint RPC because it's wrapped by the SDK's gRPC server. Node operators and developers might still use the Tendermint RPC for debugging and monitoring purposes and non-sdk chains will probably also still use the RPC. Does that provide a little extra context?

For now, the IBC will still be using the Tendermint RPC not for the verification of headers but to read the application state, fetch the pending IBX transactions and then submitting those transactions on the destination chain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess that makes sense, though perhaps it makes more sense to have (for these cases) the light client implemented on top of the SDKs grpc end points rather than based on the p2p layer?

If the goal is reduce operational complexity for relayers, then needing them to have fewer connections total, is perhaps the real solution.

Copy link
Contributor

Choose a reason for hiding this comment

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

the light client implemented on top of the SDKs grpc end points rather than based on the p2p layer?

This would make sense. Have a look at this: cosmos/cosmos-sdk#11012

Comment on lines +93 to +95
Client side light client (e.g. wallets, etc.) users may always want to use (a
subset) of the RPC rather than connect to the P2P network for an ephemeral
use.
Copy link
Contributor

Choose a reason for hiding this comment

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

Mentally, I've always made the division between RPC and P2P as a matter of nodes (and light client is a type of node) talk to each other using P2P and external clients use RPC. However, I feel making the distinction a matter of ephemeral usage (RPC) vs constant usage (P2P) as also being valid.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess a good question might therefore be does LibP2P handle ephemeral usage well. Is it easy to be connected to 1000 clients who only infrequently send requests

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see you raise that point later in the discussion section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that libp2p in general does a good job of reusing connections and managing resources (because the networks don't need to be quite as connected as our current system requires, so I think it would be ok, though we could probably tolerate some more idle connection.

docs/rfc/rfc-010-p2p-light-client.rst Show resolved Hide resolved
docs/rfc/rfc-010-p2p-light-client.rst Show resolved Hide resolved
docs/rfc/rfc-010-p2p-light-client.rst Outdated Show resolved Hide resolved
Comment on lines 121 to 127
Report Evidence
~~~~~~~~~~~~~~~

The current light client implementation currently has the ability to report
observed evidence. Either the notional light client reactor needs to be able
to handle these kinds of requests *or* all light client nodes need to also run
the evidence reactor. This could be configured at runtime.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a great point. The light client isn't able to verify most evidence but it can still generate as well as relay evidence to the network. There was also previous talk about designing a protocol such that light clients could listen out for possible attacks and freeze (like IBC does). So it will need to have some form of evidence reactor but similarly to the light reactor it will be different to the full node implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you had the full node's evidence reactor (in go; optionally) you could submit evidence, and that doesn't seem like a terrible idea?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup. I think part of having a p2p implementation would consist of submitting evidence to full nodes via the evidence channel

Similarly, libp2p makes it possible for project to be able back their non-Go
light clients, without the major task of first implementing tendermint's p2p
connection handling. We should identify if there exist users (e.g. the go IBC
relayer, it's maintainers, and operators) who would be able to take advantage
Copy link
Contributor

Choose a reason for hiding this comment

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

I talked to @jackzampolin about this feature a few times and they would love to incorporate this. The only missing part is a way to submit txs and query through the p2p based light client.

For submitting txs this could be done on the mempool connection, but query would need to be something new.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There aren't separate or per-reactor connections in the p2p layer so we'd need to build something out for this, and while we could add a "submit transaction over the p2p layer" (and perhaps should, I think this is a reasonable feature).

I'd also be super curious to know what the number of deployments of the go relayer are, and getting much more information about that roadmap.

Co-authored-by: Callum Waters <cmwaters19@gmail.com>
Co-authored-by: M. J. Fromberger <michael.j.fromberger@gmail.com>
Copy link
Contributor

@williambanfield williambanfield left a comment

Choose a reason for hiding this comment

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

The main thing that this doc makes me wonder is if we have a well-known taxonomy of light-client uses. Is it just wallets and relayers, or are there other current or planned usese?

@tychoish
Copy link
Contributor Author

I think this is a very good synopsis of the design space. Assuming consensus, is the idea that we'd do this as part of the libp2p migration? (Or at least part-concurrently with it)

I think the scheduling is a good question. It's in this interesting space because part of the work (e.g. adding a reactor to provide the data) is really orthogonal to libp2p, and so we could do it really soong. At the same time, there are challenges (e.g. having nodes that are light client not lead to DoS attacks on the p2p stack is a bit sticky, and also most of the potential users of this are likely to want to wait till libp2p, so while we could move a bunch of the implementation forward it might not be worth it.

Having said that, once libp2p lands, this becomes very compelling.

@tychoish
Copy link
Contributor Author

The main thing that this doc makes me wonder is if we have a well-known taxonomy of light-client uses. Is it just wallets and relayers, or are there other current or planned usese?

I think this would be really good to collect and write up somewhere, and I don't have a good sense of things.

I suspect given the RPC restriction a lot of things that could use the light client end up running full nodes (because it's hard to get access to RPC without running your own full node,) and for many use cases you'd end up wanting to interact more with data that full nodes have.

For the wallet case, you might end up wanting to have the light client on the client side (e.g. just to verify blocks), but these are likely in most cases not going to be implemented in go (you'd want JS, wasm, or android/ios implementations). That's not a problem, but sort of backs up the "wait for libp2p and better RPC"

tychoish and others added 10 commits January 27, 2022 12:07
These options are never set to anything but the defaults, so drop the options
and inline the defaults.
…#7713)

These are only ever used with the defaults, except in our own tests.  A search
of cs.github.com shows no other callers.

The use in the test was solely to bug out the go-metrics package so its
goroutines don't trigger the leak checker. Use the package's own flag for that
purpose instead. Note that calling "Stop" on the metric helps, but is not
sufficient -- the Stop does not wait for its goroutine to exit.
- use the full key path to pass to the VerifyAbsence function
@Wondertan
Copy link

libp2p/go-libp2p#1260 is related to DoS concern on the p2p. The issue is still open but the implementation is done and integrated to their stack recently. Nice proposal btw!

@tychoish
Copy link
Contributor Author

libp2p/go-libp2p#1260 is related to DoS concern on the p2p. The issue is still open but the implementation is done and integrated to their stack recently. Nice proposal btw!

Thanks!

My fears about DoSing are really all about our current P2P implementation, and I think there are design aspects of libp2p that would obviate all of my current concerns, but it's really nice to see that libp2p is thinking a lot about DoS mitigation.

docs/rfc/rfc-010-p2p-light-client.rst Outdated Show resolved Hide resolved
Co-authored-by: Callum Waters <cmwaters19@gmail.com>
@tychoish tychoish merged commit 2c074e2 into tendermint:master Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants