Skip to content

Commit

Permalink
Merge pull request cosmos#351 from celestiaorg/tzdybal/markdown-lint
Browse files Browse the repository at this point in the history
* add markdown linter
* fix existing markdowns
  • Loading branch information
tzdybal authored Apr 10, 2022
2 parents eae9cb5 + 4280b44 commit e917abf
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 30 deletions.
12 changes: 11 additions & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
name: golangci-lint
name: Linters
on:
push:
tags:
- v*
branches:
- main
pull_request:

jobs:
golangci:
name: lint
Expand Down Expand Up @@ -38,3 +39,12 @@ jobs:

# Optional: if set to true then the action don't cache or restore ~/.cache/go-build.
# skip-build-cache: true
markdownlint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: markdownlint-cli
uses: nosborn/github-action-markdown-cli@v3.0.1
with:
files: .
config-file: .markdownlint.yaml
7 changes: 7 additions & 0 deletions .markdownlint.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
default: true
MD010:
code_blocks: false
MD013: false
MD024:
allow_different_nesting: true

6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
Minor bugfix release, to ensure that Optimint uses the same version of gRPC as Cosmos SDK.

### BUG FIXES

- Use google.golang.org/grpc v1.33.2 to be compatible with cosmos-sdk ([#315](https://github.com/celestiaorg/optimint/pull/315)) [@jbowen93](https://github.com/jbowen93/)
- Make TestValidatorSetHandling even more stable ([#314](https://github.com/celestiaorg/optimint/pull/314)) [@tzdybal](https://github.com/tzdybal/)

Expand All @@ -14,6 +15,7 @@ This is the first Optimint release.
Optimint supports all ABCI methods and all Tendermint RPCs.

### FEATURES

- Minimal implementation of ConsensusParams method ([#292](https://github.com/celestiaorg/optimint/pull/292)) [@tzdybal](https://github.com/tzdybal/)
- Implement GenesisChunked method ([#287](https://github.com/celestiaorg/optimint/pull/287)) [@mauriceLC92](https://github.com/mauriceLC92/)
- Minimalistic validator set handling ([#286](https://github.com/celestiaorg/optimint/pull/286)) [@tzdybal](https://github.com/tzdybal/)
Expand Down Expand Up @@ -61,6 +63,7 @@ Optimint supports all ABCI methods and all Tendermint RPCs.
- Add design doc to readme ([#9](https://github.com/celestiaorg/optimint/pull/9)) [@musalbas](https://github.com/musalbas/)

### IMPROVEMENTS

- Remove extra variable ([#280](https://github.com/celestiaorg/optimint/pull/280)) [@Raneet10](https://github.com/Raneet10/)
- Replace tm-db dependency with store package ([#268](https://github.com/celestiaorg/optimint/pull/268)) [@tzdybal](https://github.com/tzdybal/)
- Use enum instead of strings for DB type ([#259](https://github.com/celestiaorg/optimint/pull/259)) [@adlerjohn](https://github.com/adlerjohn/)
Expand All @@ -87,6 +90,7 @@ Optimint supports all ABCI methods and all Tendermint RPCs.
- Use addresses in multiaddr format. ([#19](https://github.com/celestiaorg/optimint/pull/19)) [@tzdybal](https://github.com/tzdybal/)

### BUG FIXES

- fix: make `TestValidatorSetHandling` stable ([#313](https://github.com/celestiaorg/optimint/pull/313)) [@tzdybal](https://github.com/tzdybal/)
- Fix linter on `main` ([#308](https://github.com/celestiaorg/optimint/pull/308)) [@tzdybal](https://github.com/tzdybal/)
- Fix multiple bugs for Ethermint ([#305](https://github.com/celestiaorg/optimint/pull/305)) [@tzdybal](https://github.com/tzdybal/)
Expand All @@ -101,4 +105,4 @@ Optimint supports all ABCI methods and all Tendermint RPCs.
- Fix typos in node/node.go ([#86](https://github.com/celestiaorg/optimint/pull/86)) [@tzdybal](https://github.com/tzdybal/)
- Fixing linter errors ([#55](https://github.com/celestiaorg/optimint/pull/55)) [@tzdybal](https://github.com/tzdybal/)
- Add peer discovery ([#17](https://github.com/celestiaorg/optimint/pull/17)) [@tzdybal](https://github.com/tzdybal/)
- P2P bootstrapping ([#14](https://github.com/celestiaorg/optimint/pull/14)) [@tzdybal](https://github.com/tzdybal/)
- P2P bootstrapping ([#14](https://github.com/celestiaorg/optimint/pull/14)) [@tzdybal](https://github.com/tzdybal/)
22 changes: 14 additions & 8 deletions docs/lazy-adr/adr-001-node-interface.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,41 +3,47 @@
Replacing on the `Node` level gives much flexibility. Still, significant amount of code can be reused, and there is no need to refactor lazyledger-core.
Cosmos SDK is tigtly coupled with Tendermint with regards to node creation, RPC, app initialization, etc. De-coupling requires big refactoring of cosmos-sdk.

There are known issues related to Tendermint RPC communication.
There are known issues related to Tendermint RPC communication.

## Replacing Tendermint `Node`

Tendermint `Node` is a struct. It's used directly in cosmos-sdk (not via interface).
We don't need to introduce common interface `Node`s, because the plan is to use scafolding tool in the feature, so we can make any required changes in cosmos-sdk.
### Interface required by cosmos-sdk:

### Interface required by cosmos-sdk

* BaseService (struct):
* Service (interface)
* Start()
* IsRunning()
* Stop()
* Start()
* IsRunning()
* Stop()
* Logger
* Direct access:
* ConfigureRPC()
* EventBus()

## Alternative approaches

### Create RPC from scratch

* Pros:
* May be possible to avoid Tendermint issues
* Should be possible to avoid dependency on Tendermint in Optimint
* Changes probably limited to cosmos-sdk (not required in tendermint/lazyledger-core)
* Changes probably limited to cosmos-sdk (not required in tendermint/lazyledger-core)
* Cons:
* Reinventing the wheel
* Requires bigger, much more complicated changes in cosmos-sdk
* Probably can't upstream such changes to cosmos-sdk

## `tendermint` vs `lazyledger-core`
Right now, either `tendermint` or `lazyledger-core` can be used for base types (including interfaces).

Right now, either `tendermint` or `lazyledger-core` can be used for base types (including interfaces).
Similarly, vanilla `cosomos-sdk` (not a fork under lazyledger organization) can be used as a base for ORU client.
`lazyledger-core` is a repository created because of needs related to lazyledger client, not optimistic rollups client.
On the other hand, some of the functionality will be shared between both clients. This will have to be resolved later in time.
Using 'vanilla' repositories (not forks) probably will make easier to upstream changes if required, and will make scaffolding
easier.

## Development
For development, there is `master-optimint` branch in `cosmos-sdk` repository. Versions with `-optimint` suffix will be released from this branch for easier dependency management during development.

For development, there is `master-optimint` branch in `cosmos-sdk` repository. Versions with `-optimint` suffix will be released from this branch for easier dependency management during development.
3 changes: 3 additions & 0 deletions docs/lazy-adr/adr-002-mempool.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
For now, mempool implementation from lazyledger-core/Tendermint will be used.

## Pros

* good integration with other re-used code (see ADR-001)
* well tested
* glue code is not required
Expand All @@ -11,12 +12,14 @@ For now, mempool implementation from lazyledger-core/Tendermint will be used.
* mempool does not require any knowledge about the internal structure of the Txs and is already "abci-ready"

## Cons

* inherit all limitations of the tendermint mempool
* no prioritization of Txs
* many [open issues](https://github.com/tendermint/tendermint/issues?q=is%3Aissue+is%3Aopen+mempool+label%3AC%3Amempool)
* legacy code base (the tendermint mempool exists for a while now)

## Alternatives

* Implementation from scratch
* time consuming
* error prone
Expand Down
4 changes: 4 additions & 0 deletions docs/lazy-adr/adr-003-peer-discovery.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
Libp2p provides multiple ways to discover peers (DHT, mDNS, PubSub peer exchange). Currently there are no plans to support mDNS (as it's limited to local networks).

## Proposed network architecture

1. There will be a set of well-known, application-agnostic seed nodes. Every optimint client will be able to connect to such node, addresses will be saved in configuration.
* This does not limit applications as they can still create independent networks with separate set of seed nodes.
2. Nodes in the network will serve DHT. It will be used for active peer discovery. Client of each ORU network will be able to find other peers in this particular network.
Expand All @@ -12,13 +13,16 @@ Libp2p provides multiple ways to discover peers (DHT, mDNS, PubSub peer exchange
4. After connecting to nodes found in DHT, GossipSub will handle peer lists for clients.

### Pros

* Shared DHT should make it easier to find peers.
* Use of existing libraries.

### Cons

* There may be some overhead for clients to handle DHT requests from other ORU networks.

## Alternatives

1. Joining public IPFS DHT for peer discovery.
* pros: large network - finding peers should be very easy
* cons: we may affect public IPFS network stability in case of misconfiguration, possibly lot of unrelated traffic
Expand Down
25 changes: 11 additions & 14 deletions docs/lazy-adr/adr-004-core-types.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ This document describes the core data structures of any Optimint-powered blockch
## Alternative Approaches

Alternatives for ChainID:
- an integer type like unit64
- a string that fulfills some basic rules like the ChainID for Cosmos chains

- an integer type like unit64
- a string that fulfills some basic rules like the ChainID for Cosmos chains

## Decision

We design the core data types as minimalistic as possible, i.e. they only contain the absolute necessary
data for an optimistic rollup to function properly.
If there are any additional fields that conflict with above's claimed minimalism, then they are necessarily inherited
We design the core data types as minimalistic as possible, i.e. they only contain the absolute necessary
data for an optimistic rollup to function properly.
If there are any additional fields that conflict with above's claimed minimalism, then they are necessarily inherited
by the ABCI imposed separation between application state machine and consensus/networking (often also referred to as ABCI-server and -client).
Where such tradeoffs are made, we explicitly comment on them.

Expand Down Expand Up @@ -104,11 +105,11 @@ type EvidenceData struct {
The details for this will be defined in a separated adr/PR.

Here is an incomplete list of potenial evidence types:

- Same Aggregator signed two different blocks at the same height
- figure out if this is actually malicious / slashable behaviour - e.g. clients could simply accept the last block included in a LL block
- State Transition Fraud Proofs (for previous blocks)


#### Commit

```go
Expand All @@ -122,15 +123,14 @@ type Commit struct {
#### ConsensusParams

[ConsensusParams](https://docs.tendermint.com/master/spec/core/state.html#consensusparams) can be updated by the application through ABCI.
This could be seen as a state transition and the ConsensusHash in the header would then require a dedicated state fraud proof.
This could be seen as a state transition and the ConsensusHash in the header would then require a dedicated state fraud proof.
That said, none of the existing default Cosmos-SDK modules actually make use of this functionality though.
Hence, we can treat the ConsensusParams as constants (for the same app version).
We clearly need to communicate this to optimistic rollup chain developers.
Hence, we can treat the ConsensusParams as constants (for the same app version).
We clearly need to communicate this to optimistic rollup chain developers.
Ideally, we should ensure this programmatically to guarantee that this assumption always holds inside optimint.

The ConsensusParams have the exact same structure as in Tendermint. For the sake of self-containedness we still list them here:


```go
// ConsensusParams contains consensus critical parameters that determine the
// validity of blocks.
Expand Down Expand Up @@ -196,7 +196,6 @@ For finishing the implementation these items need to be tackled at least:
- [ ] equivalent types for serialization purposes (probably protobuf)
- [ ] conversion from and to protobuf


## Consequences

### Positive
Expand All @@ -211,6 +210,4 @@ For finishing the implementation these items need to be tackled at least:

## References

- https://github.com/celestiaorg/optimint/pull/41


- <https://github.com/celestiaorg/optimint/pull/41>
12 changes: 7 additions & 5 deletions docs/lazy-adr/adr-005-serialization.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ There are countless alternatives to `protobuf`, including `flatbuffers`, `avro`,

## Decision

`protobuf` is used for data serialization both for storing and network communication.
`protobuf` is used for data serialization both for storing and network communication.
`protobuf` is used widely in entire Cosmos ecosystem, and we would need to use it anyways.

## Status
Expand All @@ -24,12 +24,14 @@ There are countless alternatives to `protobuf`, including `flatbuffers`, `avro`,
## Consequences

### Positive
* well known serialization method
* language independent

- well known serialization method
- language independent

### Negative
* there are known issues with `protobuf`

- there are known issues with `protobuf`

### Neutral
* it's de-facto standard in Cosmos ecosystem

- it's de-facto standard in Cosmos ecosystem
2 changes: 1 addition & 1 deletion docs/lazy-adr/adr-006-da-interface.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ All the details are implementation-specific.
## Detailed Design

Definition of interface:

```go
type DataAvailabilityLayerClient interface {
// Init is called once to allow DA client to read configuration and initialize resources.
Expand Down Expand Up @@ -109,4 +110,3 @@ Implemented
## References

> Are there any relevant PR comments, issues that led up to this, or articles referenced for why we made the given design choice? If so link them here!

0 comments on commit e917abf

Please sign in to comment.