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

Repositories structuring #1

Closed
musalbas opened this issue Mar 26, 2020 · 7 comments
Closed

Repositories structuring #1

musalbas opened this issue Mar 26, 2020 · 7 comments

Comments

@musalbas
Copy link
Member

musalbas commented Mar 26, 2020

In building LazyLedger, we would like to use Tendermint for two purposes, that are somewhat independent from each other, that requires a different set of modifications:

  1. Optimistic rollups. A version of Tendermint that allows developers to deploy chains that can be used as optimistic rollup sidechains. In order to achieve this, the block header for Tendermint needs to be modified so that there is only one Merkle root that represents the state of the chain, so that state transition fraud proofs can be generated from this root. That means any other root that is derived from the state of the chain, such as the validator set root, must be folded in to this root. Additionally, the state root needs to be structured in such a way so that it can support fraud proofs, which would involve adding intermediate state roots to transactions.
  2. LazyLedger layer 1. We also want to use Tendermint to drive the core LazyLedger protocol. This means we need to modify the block header to support data availability proofs, as well as creating a storage node library, etc. However, the LazyLedger layer 1 will also have a consensus-critical execution environment that drives the LazyLedger token and the proof-of-stake protocol. This execution environment will be an optimistic rollup-style environment that supports state transition fraud proofs too, so it will need to adapt the block header modifications from the optimistic rollup version of Tendermint we create.

The Tendermint features we need in (1) are a subset of the ones we need in (2). We need to think about what would be the best way to structure these repositories, and if we want to have one or two forks of Tendermint.

The two options are:

  • Have one version of Tendermint both for LazyLedger core, that also allows people to build optimistic rollup chains on Tendermint. Would this get too messy though, and what would the developer experience look like? And could we make it so that people can build optimistic rollup chains via Tendermint without LazyLedger?
  • Have a separate version of Tendermint for optimistic rollup chains, that could be a general-purpose optimistic rollup implementation, that could eventually get merged back upstream. The benefit of this could be that it could be the Tendermint community de-facto codebase Tendermint chains that support fraud proofs.
@musalbas
Copy link
Member Author

CC'ing @gakonst - what modifications are you planning to make to Cosmos/Tendermint in order to enable state transition fraud proofs?

@liamsi
Copy link
Member

liamsi commented Mar 26, 2020

I wish tendemint would allow devs to plug in their own Header definitions and the header used inside of tendermint (for everything besides encoding and other places you'd need to deal with the concrete impl) would just be an interface (or trait). Similar to what we did in the Rust implementation of the light client: https://github.com/informalsystems/tendermint-rs/blob/471542d60d542a8049c81286dff13ffa467300fc/tendermint/src/lite/types.rs#L19-L31
But with additional methods like is_valid (and a few others) to be able to define your own block validity rule(s). This would allow tendermint to be used in a much more flexible manner. Similar this would mean, devs can also define their own way to merkelize transactions (instead of currently via SimpleHashFromByteSlices).

Would the tendermint team like to integrate changes in this direction? @tessr @marbar3778

@melekes
Copy link
Contributor

melekes commented Apr 1, 2020

I can't speak for the tendermint team, but I'd like to see Tendermint more flexible to allow experiments / modifications like ^ without too much pain 👍

@tac0turtle
Copy link
Member

I wish tendemint would allow devs to plug in their own Header definitions and the header used inside of tendermint

This is the goal for Tendermint. I hope with the move to protobuf Tendermint can better facilitate this modularity.

@liamsi
Copy link
Member

liamsi commented Apr 1, 2020

Thanks for your feedback @melekes and @marbar3778. And cool, would be great if we could help tendermint to reach that goal even further :-)


Making tendermint as flexible as mentioned above will take some time. So just summarizing what we've discussed as possible options for the meantime:

@adlerjohn suggested we can use the AppHash via Commit in the (tendermint/abci) app for the state root mentioned in 1. (optimistic rollups). The state root could be structured in any desired way (it's completely up to the app; e.g. the cosmos hub uses an iavl tree; well actually several such trees via a MultiStore).

@musalbas raised concerns about this approach as it is actually necessary that there is only one and only one data/state root that includes all consensus critical data. The reason for this is that otherwise we can't guarantee that the data behind the other roots is actually available. A good alternative seems to be to do all modifications in one fork and make the changes needed for 2. configurable somehow. But as 2. involves changes to the header (add the erasure coded merkle tree data availability scheme), this only makes sense if the header is modular/pluggable in some sense (which kinda brings us back to #1 (comment)).

liamsi pushed a commit that referenced this issue Apr 4, 2020
* rename adjusted to adjacent

Refs tendermint/tendermint#3989 (comment)

* rename ErrTooMuchChange to ErrNotEnoughVotingPowerSigned

Refs tendermint/tendermint#3989 (comment)

* verify commit is properly signed

* remove no longer trusted headers

* restore trustedHeader and trustedNextVals

* check trustedHeader using options

Refs tendermint/tendermint#4209 (comment)

* use correct var when checking if headers are adjacent

in bisection func
+ replace TODO with a comment

tendermint/tendermint#3989 (comment)

* return header in VerifyHeaderAtHeight

because that way we avoid DB call

+ add godoc comments
+ check if there are no headers yet in AutoClient

tendermint/tendermint#3989 (review)

* TestVerifyAdjacentHeaders: add 2 more test-cases

+ add TestVerifyReturnsErrorIfTrustLevelIsInvalid

* lite: avoid overflow when parsing key in db store!

* lite: rename AutoClient#Err to Errs

* lite: add a test for AutoClient

* lite: fix keyPattern and call itr.Next in db store

* lite: add two tests for db store

* lite: add TestClientRemovesNoLongerTrustedHeaders

* lite: test Client#Cleanup

* lite: test restoring trustedHeader

tendermint/tendermint#4209 (comment)

* lite: comment out unused code in test_helpers

* fix TestVerifyReturnsErrorIfTrustLevelIsInvalid after merge

* change defaultRemoveNoLongerTrustedHeadersPeriod

and add docs

* write more doc

* lite: uncomment testable examples

* use stdlog.Fatal to stop AutoClient tests

* make lll linter happy

* separate errors for 2 cases

- the validator set of a skipped header cannot be trusted, i.e. <1/3rd
  of h1 validator set has signed (new error, something like
  ErrNewValSetCantBeTrusted)
- the validator set is trusted but < 2/3rds has signed
  (ErrNewHeaderCantBeTrusted)

tendermint/tendermint#4209 (comment)

* remove all headers (even the last one) that are outside

of the trusting period. By doing this, we avoid checking the
trustedHeader's hash in checkTrustedHeaderUsingOptions (case #1).

tendermint/tendermint#4209 (comment)

* explain restoreTrustedHeaderAndNextVals better

tendermint/tendermint#4209 (comment)

* add ConfirmationFunction option

for optionally prompting for user input Y/n before removing headers

Refs tendermint/tendermint#4209 (comment)

* make cleaning optional

tendermint/tendermint#4209 (comment)

* return error when user refused to remove headers

* check for double votes in VerifyCommitTrusting

* leave only ErrNewValSetCantBeTrusted error

to differenciate between h2Vals.VerifyCommit and
h1NextVals.VerifyCommitTrusting

* fix example tests

* remove unnecessary if condition

tendermint/tendermint#4209 (comment)

It will be handled by the above switch.

* verifyCommitBasic does not depend on vals

Co-authored-by: Marko <marbar3778@yahoo.com>
@liamsi liamsi added this to To do in LazyLedger MVP via automation Apr 22, 2020
@liamsi liamsi moved this from To do to Low-Prio Backlog in LazyLedger MVP Apr 22, 2020
@liamsi liamsi moved this from Low-Prio Backlog to In progress in LazyLedger MVP Apr 22, 2020
@liamsi
Copy link
Member

liamsi commented Apr 22, 2020

There are still open questions or todos around this issue right?

We didn't fully settle on this?

And could we make it so that people can build optimistic rollup chains via Tendermint without LazyLedger?

Although I'm not sure if most that code would't live as a cosmos-sdk module outside of tendermint anyways.

And merging all other roots into a single state root:

That means any other root that is derived from the state of the chain, such as the validator set root, must be folded in to this root.

I mean it is also the job of the app to include all state in AppHash. No modifications in tendermint needed here. Obviously deleting the validator-roots requires changes in tendermint. So I'm still not sure what would be best. I think we should leave this issue open and proceed with tendermint-core and a cosmos-sdk app until we have more clarity around this.

@liamsi
Copy link
Member

liamsi commented Apr 22, 2020

OK, we actually concluded: one repo for now. This can be closed.

@liamsi liamsi closed this as completed Apr 22, 2020
LazyLedger MVP automation moved this from In progress to Done Apr 22, 2020
@hoangdv2429 hoangdv2429 mentioned this issue Dec 11, 2023
3 tasks
staheri14 added a commit that referenced this issue Jan 23, 2024
…heck (#1186)

This PR addresses the vulnerabilities identified by govulncheck in [PR
#1179](https://github.com/celestiaorg/celestia-core/actions/runs/7629759074/job/20786518441?pr=1179).
It upgrades the affected modules to the versions recommended by
govulncheck.
```
Vulnerability #1: GO-2024-2466
    Denial of service in github.com/go-git/go-git/v5 and
    gopkg.in/src-d/go-git.v4
  More info: https://pkg.go.dev/vuln/GO-2024-2466
  Module: github.com/go-git/go-git/v5
    Found in: github.com/go-git/go-git/v5@v5.5.1
    Fixed in: github.com/go-git/go-git/v5@v5.11.0
    Example traces found:
Error:       #1: test/e2e/generator/generate.go:402:36: generator.gitRepoLatestReleaseVersion calls git.PlainOpenWithOptions, which calls filesystem.NewStorage
Error:       #2: test/e2e/generator/generate.go:402:36: generator.gitRepoLatestReleaseVersion calls git.PlainOpenWithOptions
Error:       #3: test/e2e/generator/generate.go:407:30: generator.gitRepoLatestReleaseVersion calls git.Repository.TagObjects

Vulnerability #2: GO-2024-2456
    Path traversal and RCE in github.com/go-git/go-git/v5 and
    gopkg.in/src-d/go-git.v4
  More info: https://pkg.go.dev/vuln/GO-2024-2456
  Module: github.com/go-git/go-git/v5
    Found in: github.com/go-git/go-git/v5@v5.5.1
    Fixed in: github.com/go-git/go-git/v5@v5.11.0
    Example traces found:
Error:       #1: test/e2e/generator/generate.go:402:36: generator.gitRepoLatestReleaseVersion calls git.PlainOpenWithOptions, which calls filesystem.NewStorage
Error:       #2: test/e2e/generator/generate.go:402:36: generator.gitRepoLatestReleaseVersion calls git.PlainOpenWithOptions
Error:       #3: test/e2e/generator/generate.go:407:30: generator.gitRepoLatestReleaseVersion calls git.Repository.TagObjects

=== Informational ===

There are 2 vulnerabilities in modules that you require that are
neither imported nor called. You may not need to take any action.
See https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck for details.

Vulnerability #1: GO-2024-2453
    Timing side channel in github.com/cloudflare/circl
  More info: https://pkg.go.dev/vuln/GO-2024-2453
  Module: github.com/cloudflare/circl
    Found in: github.com/cloudflare/circl@v1.3.1
    Fixed in: github.com/cloudflare/circl@v1.3.7

Vulnerability #2: GO-2023-[17](https://github.com/celestiaorg/celestia-core/actions/runs/7629759074/job/20786518441?pr=1179#step:5:18)65
    Leaked shared secret and weak blinding in github.com/cloudflare/circl
  More info: https://pkg.go.dev/vuln/GO-[20](https://github.com/celestiaorg/celestia-core/actions/runs/7629759074/job/20786518441?pr=1179#step:5:21)23-1765
  Module: github.com/cloudflare/circl
    Found in: github.com/cloudflare/circl@v1.3.1
    Fixed in: github.com/cloudflare/circl@v1.3.3

Your code is affected by 2 vulnerabilities from 1 module.

Share feedback at https://go.dev/s/govulncheck-feedback.
exit status 3
make: *** [Makefile:254: vulncheck] Error 1
Error: Process completed with exit code 2.
```
staheri14 added a commit that referenced this issue Mar 11, 2024
In order to fix the go vulnerabilities that are fixed in the new patch:
```
Vulnerability #1: GO-2024-2610
    Errors returned from JSON marshaling may break template escaping in
    html/template
  More info: https://pkg.go.dev/vuln/GO-2024-2610
  Standard library
    Found in: html/template@go1.22
    Fixed in: html/template@go1.22.1
    Example traces found:
      #1: test/fuzz/rpc/jsonrpc/server/handler.go:30:15: server.Fuzz calls http.ServeMux.ServeHTTP, which eventually calls template.Template.Execute
      #2: rpc/jsonrpc/server/http_server.go:256:15: server.maxBytesHandler.ServeHTTP calls http.HandlerFunc.ServeHTTP, which eventually calls template.Template.ExecuteTemplate

Vulnerability #2: GO-2024-2600
    Incorrect forwarding of sensitive headers and cookies on HTTP redirect in
    net/http
  More info: https://pkg.go.dev/vuln/GO-2024-2600
  Standard library
    Found in: net/http@go1.22
    Fixed in: net/http@go1.22.1
    Example traces found:
      #1: rpc/jsonrpc/client/http_json_client.go:213:34: client.Client.Call calls http.Client.Do
      #2: libs/cli/setup.go:89:26: cli.Executor.Execute calls cobra.Command.Execute, which eventually calls http.Client.Get
      #3: p2p/upnp/upnp.go:205:20: upnp.getServiceURL calls http.Get

Vulnerability #3: GO-2024-2599
    Memory exhaustion in multipart form parsing in net/textproto and net/http
  More info: https://pkg.go.dev/vuln/GO-2024-2599
  Standard library
    Found in: net/textproto@go1.22
    Fixed in: net/textproto@go1.22.1
    Example traces found:
      #1: rpc/jsonrpc/server/http_server.go:62:16: server.Serve calls http.Server.Serve, which eventually calls textproto.Reader.ReadLine
      #2: rpc/jsonrpc/server/http_server.go:62:16: server.Serve calls http.Server.Serve, which eventually calls textproto.Reader.ReadMIMEHeader

Vulnerability #4: GO-2024-2598
    Verify panics on certificates with an unknown public key algorithm in
    crypto/x509
  More info: https://pkg.go.dev/vuln/GO-2024-2598
  Standard library
    Found in: crypto/x509@go1.22
    Fixed in: crypto/x509@go1.22.1
    Example traces found:
      #1: libs/autofile/group.go:479:30: autofile.GroupReader.Read calls bufio.Reader.Read, which eventually calls x509.Certificate.Verify

Your code is affected by 4 vulnerabilities from the Go standard library.

```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

4 participants