From 128678588e76a9a440ffb0254aaae4d73dc7731a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Zdyba=C5=82?= Date: Sun, 10 Apr 2022 16:24:17 +0200 Subject: [PATCH 1/5] ci: add markdown linter --- .github/workflows/lint.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 8fe1c6692..6993a73f1 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,4 +1,4 @@ -name: golangci-lint +name: Linters on: push: tags: @@ -6,6 +6,7 @@ on: branches: - main pull_request: + jobs: golangci: name: lint @@ -38,3 +39,11 @@ 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: . \ No newline at end of file From ce814886083cf29c6be5c109b4c1a840ba1600fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Zdyba=C5=82?= Date: Sun, 10 Apr 2022 16:43:56 +0200 Subject: [PATCH 2/5] fix: markdownlint -fix . --- CHANGELOG.md | 6 ++- docs/lazy-adr/adr-001-node-interface.md | 22 +++++---- docs/lazy-adr/adr-002-mempool.md | 3 ++ docs/lazy-adr/adr-003-peer-discovery.md | 4 ++ docs/lazy-adr/adr-004-core-types.md | 25 +++++----- docs/lazy-adr/adr-005-serialization.md | 12 +++-- docs/lazy-adr/adr-006-da-interface.md | 64 ++++++++++++------------- 7 files changed, 76 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75aa2491d..653ea34b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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/) @@ -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/) @@ -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/) @@ -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/) @@ -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/) \ No newline at end of file +- P2P bootstrapping ([#14](https://github.com/celestiaorg/optimint/pull/14)) [@tzdybal](https://github.com/tzdybal/) diff --git a/docs/lazy-adr/adr-001-node-interface.md b/docs/lazy-adr/adr-001-node-interface.md index 8474e58d5..8ba5a0901 100644 --- a/docs/lazy-adr/adr-001-node-interface.md +++ b/docs/lazy-adr/adr-001-node-interface.md @@ -3,35 +3,41 @@ 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. @@ -39,5 +45,5 @@ Using 'vanilla' repositories (not forks) probably will make easier to upstream c 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. diff --git a/docs/lazy-adr/adr-002-mempool.md b/docs/lazy-adr/adr-002-mempool.md index a9844f7c8..a0c141419 100644 --- a/docs/lazy-adr/adr-002-mempool.md +++ b/docs/lazy-adr/adr-002-mempool.md @@ -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 @@ -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 diff --git a/docs/lazy-adr/adr-003-peer-discovery.md b/docs/lazy-adr/adr-003-peer-discovery.md index 123c50977..92d723251 100644 --- a/docs/lazy-adr/adr-003-peer-discovery.md +++ b/docs/lazy-adr/adr-003-peer-discovery.md @@ -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. @@ -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 diff --git a/docs/lazy-adr/adr-004-core-types.md b/docs/lazy-adr/adr-004-core-types.md index c5be75edb..3329437e6 100644 --- a/docs/lazy-adr/adr-004-core-types.md +++ b/docs/lazy-adr/adr-004-core-types.md @@ -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. @@ -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 @@ -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. @@ -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 @@ -211,6 +210,4 @@ For finishing the implementation these items need to be tackled at least: ## References -- https://github.com/celestiaorg/optimint/pull/41 - - +- diff --git a/docs/lazy-adr/adr-005-serialization.md b/docs/lazy-adr/adr-005-serialization.md index 08f57c28a..2e94a80b6 100644 --- a/docs/lazy-adr/adr-005-serialization.md +++ b/docs/lazy-adr/adr-005-serialization.md @@ -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 @@ -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 diff --git a/docs/lazy-adr/adr-006-da-interface.md b/docs/lazy-adr/adr-006-da-interface.md index c97e5ee32..6604f7e5c 100644 --- a/docs/lazy-adr/adr-006-da-interface.md +++ b/docs/lazy-adr/adr-006-da-interface.md @@ -24,27 +24,28 @@ 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. - Init(config []byte, kvStore store.KVStore, logger log.Logger) error + // Init is called once to allow DA client to read configuration and initialize resources. + Init(config []byte, kvStore store.KVStore, logger log.Logger) error - Start() error - Stop() error + Start() error + Stop() error - // SubmitBlock submits the passed in block to the DA layer. - // This should create a transaction which (potentially) - // triggers a state transition in the DA layer. - SubmitBlock(block *types.Block) ResultSubmitBlock + // SubmitBlock submits the passed in block to the DA layer. + // This should create a transaction which (potentially) + // triggers a state transition in the DA layer. + SubmitBlock(block *types.Block) ResultSubmitBlock - // CheckBlockAvailability queries DA layer to check block's data availability. - CheckBlockAvailability(block *types.Block) ResultCheckBlock + // CheckBlockAvailability queries DA layer to check block's data availability. + CheckBlockAvailability(block *types.Block) ResultCheckBlock } // BlockRetriever is additional interface that can be implemented by Data Availability Layer Client that is able to retrieve // block data from DA layer. This gives the ability to use it for block synchronization. type BlockRetriever interface { - RetrieveBlock(height uint64) ResultRetrieveBlock + RetrieveBlock(height uint64) ResultRetrieveBlock } // TODO define an enum of different non-happy-path cases @@ -54,40 +55,40 @@ type StatusCode uint64 // Data Availability return codes. const ( - StatusUnknown StatusCode = iota - StatusSuccess - StatusTimeout - StatusError + StatusUnknown StatusCode = iota + StatusSuccess + StatusTimeout + StatusError ) type DAResult struct { - // Code is to determine if the action succeeded. - Code StatusCode - // Message may contain DA layer specific information (like DA block height/hash, detailed error message, etc) - Message string + // Code is to determine if the action succeeded. + Code StatusCode + // Message may contain DA layer specific information (like DA block height/hash, detailed error message, etc) + Message string } // ResultSubmitBlock contains information returned from DA layer after block submission. type ResultSubmitBlock struct { - DAResult - // Not sure if this needs to be bubbled up to other - // parts of Optimint. - // Hash hash.Hash + DAResult + // Not sure if this needs to be bubbled up to other + // parts of Optimint. + // Hash hash.Hash } // ResultCheckBlock contains information about block availability, returned from DA layer client. type ResultCheckBlock struct { - DAResult - // DataAvailable is the actual answer whether the block is available or not. - // It can be true if and only if Code is equal to StatusSuccess. - DataAvailable bool + DAResult + // DataAvailable is the actual answer whether the block is available or not. + // It can be true if and only if Code is equal to StatusSuccess. + DataAvailable bool } type ResultRetrieveBlock struct { - DAResult - // Block is the full block retrieved from Data Availability Layer. - // If Code is not equal to StatusSuccess, it has to be nil. - Block *types.Block + DAResult + // Block is the full block retrieved from Data Availability Layer. + // If Code is not equal to StatusSuccess, it has to be nil. + Block *types.Block } ``` > @@ -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! - From 2a063f417d1ca7565c1dd7eee83719d4dfcfc19f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Zdyba=C5=82?= Date: Sun, 10 Apr 2022 17:10:30 +0200 Subject: [PATCH 3/5] feat: disable MD013 (line length) check --- .github/workflows/lint.yml | 3 ++- .markdownlint.yaml | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 .markdownlint.yaml diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 6993a73f1..8c3249636 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -46,4 +46,5 @@ jobs: - name: markdownlint-cli uses: nosborn/github-action-markdown-cli@v3.0.1 with: - files: . \ No newline at end of file + files: . + config-file: .markdownlint.yaml \ No newline at end of file diff --git a/.markdownlint.yaml b/.markdownlint.yaml new file mode 100644 index 000000000..128372fc9 --- /dev/null +++ b/.markdownlint.yaml @@ -0,0 +1,3 @@ +default: true +MD013: false + From f1ae4765f5e8d96f0798fdf8a32635fa663175d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Zdyba=C5=82?= Date: Sun, 10 Apr 2022 17:13:54 +0200 Subject: [PATCH 4/5] feat: set allow_different_nesting to true --- .markdownlint.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.markdownlint.yaml b/.markdownlint.yaml index 128372fc9..a163c728a 100644 --- a/.markdownlint.yaml +++ b/.markdownlint.yaml @@ -1,3 +1,5 @@ default: true MD013: false +MD024: + allow_different_nesting: true From 4280b440f7f180b7831b9128a0d08524bda01a5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Zdyba=C5=82?= Date: Sun, 10 Apr 2022 18:23:58 +0200 Subject: [PATCH 5/5] feat: allow hard tabs in code blocks in md files * disable MD010 for `code_blocks` * revert and re `markdownlint -fix` all the .md files --- .markdownlint.yaml | 2 + docs/lazy-adr/adr-006-da-interface.md | 62 +++++++++++++-------------- 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/.markdownlint.yaml b/.markdownlint.yaml index a163c728a..7d2cc8570 100644 --- a/.markdownlint.yaml +++ b/.markdownlint.yaml @@ -1,4 +1,6 @@ default: true +MD010: + code_blocks: false MD013: false MD024: allow_different_nesting: true diff --git a/docs/lazy-adr/adr-006-da-interface.md b/docs/lazy-adr/adr-006-da-interface.md index 6604f7e5c..c0cf7564d 100644 --- a/docs/lazy-adr/adr-006-da-interface.md +++ b/docs/lazy-adr/adr-006-da-interface.md @@ -27,25 +27,25 @@ Definition of interface: ```go type DataAvailabilityLayerClient interface { - // Init is called once to allow DA client to read configuration and initialize resources. - Init(config []byte, kvStore store.KVStore, logger log.Logger) error + // Init is called once to allow DA client to read configuration and initialize resources. + Init(config []byte, kvStore store.KVStore, logger log.Logger) error - Start() error - Stop() error + Start() error + Stop() error - // SubmitBlock submits the passed in block to the DA layer. - // This should create a transaction which (potentially) - // triggers a state transition in the DA layer. - SubmitBlock(block *types.Block) ResultSubmitBlock + // SubmitBlock submits the passed in block to the DA layer. + // This should create a transaction which (potentially) + // triggers a state transition in the DA layer. + SubmitBlock(block *types.Block) ResultSubmitBlock - // CheckBlockAvailability queries DA layer to check block's data availability. - CheckBlockAvailability(block *types.Block) ResultCheckBlock + // CheckBlockAvailability queries DA layer to check block's data availability. + CheckBlockAvailability(block *types.Block) ResultCheckBlock } // BlockRetriever is additional interface that can be implemented by Data Availability Layer Client that is able to retrieve // block data from DA layer. This gives the ability to use it for block synchronization. type BlockRetriever interface { - RetrieveBlock(height uint64) ResultRetrieveBlock + RetrieveBlock(height uint64) ResultRetrieveBlock } // TODO define an enum of different non-happy-path cases @@ -55,40 +55,40 @@ type StatusCode uint64 // Data Availability return codes. const ( - StatusUnknown StatusCode = iota - StatusSuccess - StatusTimeout - StatusError + StatusUnknown StatusCode = iota + StatusSuccess + StatusTimeout + StatusError ) type DAResult struct { - // Code is to determine if the action succeeded. - Code StatusCode - // Message may contain DA layer specific information (like DA block height/hash, detailed error message, etc) - Message string + // Code is to determine if the action succeeded. + Code StatusCode + // Message may contain DA layer specific information (like DA block height/hash, detailed error message, etc) + Message string } // ResultSubmitBlock contains information returned from DA layer after block submission. type ResultSubmitBlock struct { - DAResult - // Not sure if this needs to be bubbled up to other - // parts of Optimint. - // Hash hash.Hash + DAResult + // Not sure if this needs to be bubbled up to other + // parts of Optimint. + // Hash hash.Hash } // ResultCheckBlock contains information about block availability, returned from DA layer client. type ResultCheckBlock struct { - DAResult - // DataAvailable is the actual answer whether the block is available or not. - // It can be true if and only if Code is equal to StatusSuccess. - DataAvailable bool + DAResult + // DataAvailable is the actual answer whether the block is available or not. + // It can be true if and only if Code is equal to StatusSuccess. + DataAvailable bool } type ResultRetrieveBlock struct { - DAResult - // Block is the full block retrieved from Data Availability Layer. - // If Code is not equal to StatusSuccess, it has to be nil. - Block *types.Block + DAResult + // Block is the full block retrieved from Data Availability Layer. + // If Code is not equal to StatusSuccess, it has to be nil. + Block *types.Block } ``` >