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

adr: mvp light client #311

Merged
merged 30 commits into from
May 14, 2021
Merged
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
9b18942
First stab on mvp light client adr
liamsi May 3, 2021
8aabbfa
Add more details about IPFS & testing
liamsi May 4, 2021
283cfc8
Add thoughts about DAHeader via RPC
liamsi May 4, 2021
8ab6c09
minor cleanup
liamsi May 4, 2021
64fdfaf
more details on required changes:
liamsi May 4, 2021
c10756f
Update LightBlock comment
liamsi May 4, 2021
96f9903
properly introduce DAHeader abbreviation and add a negative plus some…
liamsi May 4, 2021
58b1506
Add Store requirements & note on skipping verification UX
liamsi May 5, 2021
7d06ae2
fix sentence
liamsi May 6, 2021
cef2776
Add Provider changes
liamsi May 6, 2021
9e8989b
Update docs/lazy-adr/adr-004-mvp-light-client.md
liamsi May 8, 2021
a062faf
Update docs/lazy-adr/adr-004-mvp-light-client.md
liamsi May 8, 2021
257dbbc
Update docs/lazy-adr/adr-004-mvp-light-client.md
liamsi May 8, 2021
1b119a7
Update docs/lazy-adr/adr-004-mvp-light-client.md
liamsi May 8, 2021
0369f0f
Minor changes (title & cmd help text) and moving IPFS section
liamsi May 8, 2021
eb64c8e
Remove LAZY prefix from all ADRs
liamsi May 8, 2021
eca6147
add missing word
liamsi May 8, 2021
64044aa
fix typo from review feedback
liamsi May 8, 2021
2dddade
Fix diff
liamsi May 8, 2021
a275f1e
fix other diff
liamsi May 8, 2021
f3006e2
link to DAHeader earlier
liamsi May 8, 2021
747c397
Add alternative for LightBlock and fix typo
liamsi May 8, 2021
17c9e5a
Changes to `Client` and usage of `ValidateAvailability`
liamsi May 8, 2021
7c91f68
cleanup some todos
liamsi May 8, 2021
48b0430
Add note about DHT Client/Server modes
liamsi May 8, 2021
9a08608
add numSamples to UX
liamsi May 9, 2021
7fb463c
Update docs/lazy-adr/adr-004-mvp-light-client.md
liamsi May 10, 2021
54b9b5d
Resolve a bunch of todos and fix sentence
liamsi May 10, 2021
630e967
Update docs/lazy-adr/adr-004-mvp-light-client.md
liamsi May 10, 2021
e5cf047
Update docs/lazy-adr/adr-004-mvp-light-client.md
liamsi May 10, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Resolve a bunch of todos and fix sentence
liamsi committed May 10, 2021
commit 54b9b5daaf60a23c47ab1eed417e425d31e04bee
15 changes: 7 additions & 8 deletions docs/lazy-adr/adr-004-mvp-light-client.md
Original file line number Diff line number Diff line change
@@ -179,8 +179,8 @@ Instead, adding a field to the existing `LightBlock`is backwards compatible and

##### Provider

The [`Provider`](https://github.com/tendermint/tendermint/blob/7f30bc96f014b27fbe74a546ea912740eabdda74/light/provider/provider.go#L9-L26) should be changed to enable DAS light clients to additionally
Implementations of the interface need to additionally to retrieve the DataAvailability header in the [changed LightBlock](#lightblock).
The [`Provider`](https://github.com/tendermint/tendermint/blob/7f30bc96f014b27fbe74a546ea912740eabdda74/light/provider/provider.go#L9-L26) should be changed to additionally provide the `DataAvailabilityHeader` to enable DAS light clients.
Implementations of the interface need to additionally to retrieve the `DataAvailabilityHeader` for the [modified LightBlock](#lightblock).

This comment was marked as resolved.

Copy link
Member

Choose a reason for hiding this comment

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

Or they can get it from ResultCommit.SignedHeader.Commit.BlockID.DAHeader if #304 lands before MVP, not to additional request overhead

Copy link
Member Author

@liamsi liamsi May 10, 2021

Choose a reason for hiding this comment

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

if #304 lands before MVP,

You mean #312?

Yeah, I intentionally didn't want to rely on #312. I can't think of a use-case where the DAHeader would be requested in it's own but I guess it certainly won't harm if it can be requested. We can remove the rpc endpoint / the additional method in the provider later if we wanted. It does not introduce any technical debt.

Copy link
Member

@Wondertan Wondertan May 12, 2021

Choose a reason for hiding this comment

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

You mean #312?

Yes

Copy link
Member

Choose a reason for hiding this comment

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

I can't think of a use-case where the DAHeader would be requested in it's own but I guess it certainly won't harm if it can be requested. We can remove the rpc endpoint / the additional method in the provider later if we wanted. It does not introduce any technical debt.

Ok

Users of the provider need to indicate this to the provider.

We could either augment the `LightBlock` method with a flag, add a new method solely for providing the `DataAvailabilityHeader`, or, we could introduce a new method for DAS light clients.
@@ -228,26 +228,25 @@ This means:
An example for 2. can be found in the IPFS [code](https://github.com/ipfs/go-ipfs/blob/cd72589cfd41a5397bb8fc9765392bca904b596a/cmd/ipfs/daemon.go#L239) itself.
We might want to provide a slightly different default initialization though (see how this is [overridable](https://github.com/ipfs/go-ipfs/blob/cd72589cfd41a5397bb8fc9765392bca904b596a/cmd/ipfs/daemon.go#L164-L165) in the ipfs daemon cmd).

We note that for operating a fully functional light client the IPFS node could be running in client mode [`dht.ModeClient`](TODO) but be actually want light clients to also respond to incoming queries, e.g. from other light clients.
Hence, they should by default run in `dht.ModeServer`.
We note that for operating a fully functional light client the IPFS node could be running in client mode [`dht.ModeClient`](https://github.com/libp2p/go-libp2p-kad-dht/blob/09d923fcf68218181b5cd329bf5199e767bd33c3/dht_options.go#L29-L30) but be actually want light clients to also respond to incoming queries, e.g. from other light clients.
Hence, they should by default run in [`dht.ModeServer`](https://github.com/libp2p/go-libp2p-kad-dht/blob/09d923fcf68218181b5cd329bf5199e767bd33c3/dht_options.go#L31-L32).
In an environment were any bandwidth must be saved, or, were the network conditions do not allow the server mode, we make it easy to change the default behavior.

##### Client

We add another [`Option`](TODO) to the [`Client`](TODO) that indicates that this client does DAS.
We add another [`Option`](https://github.com/tendermint/tendermint/blob/a91680efee3653e3de620f24eb8ddca1c95ce8f9/light/client.go#L43-L117) to the [`Client`](https://github.com/tendermint/tendermint/blob/a91680efee3653e3de620f24eb8ddca1c95ce8f9/light/client.go#L173) that indicates that this client does DAS.

This option indicates:
1. to do sequential verification and
2. to request [`DASLightBlocks`](#lightblock) from the [provider](#provider).

All other changes should only affect unexported methods only.
> TODO: validate this assumption via the implementation.

##### ValidateAvailability

In order for the light clients to perform DAS to validate availability, they do not need to be aware of the fact that an IPFS node is run.
Instead, we can use the existing `ValidateAvailability` function (as defined in [ADR 002](adr-002-ipld-da-sampling.md) and implemented in [#XXX](TODO)).
Note that this expects an ipfs core API object `CoreAPI`to be passed in.
Instead, we can use the existing [`ValidateAvailability`](https://github.com/lazyledger/lazyledger-core/blame/master/p2p/ipld/validate.go#L23-L28) function (as defined in [ADR 002](adr-002-ipld-da-sampling.md) and implemented in [#270](https://github.com/lazyledger/lazyledger-core/pull/270)).
Note that this expects an ipfs core API object `CoreAPI` to be passed in.
Using that interface has the major benefit that we could even change the requirement that the light client itself runs the IPFS node without changing most of the validation logic.
E.g., the IPFS node (with our custom IPLD plugin) could run in different process (or machine), and we could still just pass in that same `CoreAPI` interface.