Skip to content

spec: merge spec repo into tendermint repo#7804

Merged
cmwaters merged 264 commits intomasterfrom
callum/merge-spec
Feb 17, 2022
Merged

spec: merge spec repo into tendermint repo#7804
cmwaters merged 264 commits intomasterfrom
callum/merge-spec

Conversation

@cmwaters
Copy link
Contributor

@cmwaters cmwaters commented Feb 11, 2022

This PR looks to implement ADR 076 through the following steps:

  • Perform a rough merge by adding the spec as the remote and running git pull spec master --allow-unrelated-histories to combine commit history in parallel. Then sift through and resolve merge conflicts.
  • Remove rust-spec
  • Move spec protos to proto/spec alongside proto/tendermint
  • Clean up proto generation scripts and tooling
  • Merge any github workflows
  • Reroute any urls to the spec repo back into the tendermint repo
  • Merge RFC's adding references to the original PR's in the spec repo
  • Move the ivy-proofs within the spec repo
  • Do the same for backport branches v0.35.x and v0.34.x off the correct spec commits
  • Transfer the existing issues across and check for any permission changes

To be decided:

  • Shall the spec be in the docs directory or the root directory?

DO NOT SQUASH MERGE PLS

milosevic and others added 30 commits November 21, 2019 12:53
* reflect breaking changes made to Commit

PR: #4146
Issue: #1648

* types: rename Commit#Precommits to Signatures

* update BlockIDFlagAbsent comment

* remove iota
Co-Authored-By: Anca Zamfir <ancazamfir@users.noreply.github.com>
…DR-050 (#68)

* Add spec doc about unconditional_peer_ids, persistent_peers_max_dial_period of ADR-050

* Add indefinitely dialing condition
* sr25519 amino

* Update spec/blockchain/encoding.md

Co-Authored-By: Marko <marbar3778@yahoo.com>
…ent_algo

# Conflicts:
#	spec/consensus/light-client.md
Move light specs to their own dir, add readme and diagram
Update specification of light client algorithm to align with the code
./light -> ./light-client
- [RFC-008: Don't Panic](./rfc-008-don't-panic.md)
- [RFC-009: Consensus Parameter Upgrades](./rfc-009-consensus-parameter-upgrades.md)
- [RFC-010: P2P Light Client](./rfc-010-p2p-light-client.rst)
- [RFC-011: Block Retention](./rfc-011-block-retention.md)

Choose a reason for hiding this comment

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

Do we want these to be RFCs? It seems like spec RFCs are used more like what we call ADRs in the core.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. I don't think all the RFC's in the spec can be mapped to ADR's in Core (I think the ABCI++ RFC for example was more RFCy than others) but I think there are a few where you're right. I thought, however, it might be easier for people to find if we kept them in the RFC section of Tendermint.

Choose a reason for hiding this comment

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

Yeah—I was mainly reacting to the format of the RFC docs we're importing, which uses the same (or very similar) structure with "decision" and "pro/con/neutral" sections.

I suppose we could also keep them in spec/rfc if we aren't sure.

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 don't like the idea of having two rfc directories. I will move them across as ADR's with the exception of ABCI++ and Semantic Versioning which I think serve better as artefacts of conversations around certain problems

Copy link

@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.

This seems overall fairly safe. The main thing I'm a little concerned about is the merging of toplevel config files, scripts, etc., like Makefile and GitHub workflows. I pointed out a few things that stuck out to me, but we should be prepared for other gotchas in the build.

That said: I don't have any objection if we have to clean up some stuff after the fact.

@cmwaters cmwaters marked this pull request as ready for review February 15, 2022 10:57
@cmwaters
Copy link
Contributor Author

With the aim of trying to get the bulk of this merge swiftly through, I've marked this as ready to review. I've just done a course grain sweep of things trying to clean up existing references to the spec, merging the RFC's and documentation around the two. The generated proto files have intentionally been left untouched as there is currently a discrepancy between the spec protos due to the ABCI++ work. The generation scripts also needs to be adjusted but I'm going to push that to a separate PR.

Remaining work resides in porting across the backports and doing all the github migration stuff like bringing across the issues and signalling the migration in the tendermint/spec repo

@creachadair
Copy link

The proto-lint failures appear to be real (seems like path mismatch).

@cmwaters
Copy link
Contributor Author

The proto-lint failures appear to be real (seems like path mismatch).

Both the spec and tendermint-go kept their protos under proto/tendermint. I wanted to separate them to distinguish what was part of the specification and what was internal and specific to the go implementation but this seemed more difficult than what I first thought. To keep this PR as narrow focused as possible I have combined the directories but aim to separate them in a later PR

@creachadair
Copy link

It appears we may still be missing some proto files:

proto/tendermint/consensus/wal.proto:6:8:gogoproto/gogo.proto: does not exist
proto/tendermint/privval/service.proto:5:8:tendermint/privval/types.proto: does not exist
proto/tendermint/privval/types.proto:4:8:tendermint/crypto/keys.proto: does not exist
proto/tendermint/state/types.proto:6:8:gogoproto/gogo.proto: does not exist
proto/tendermint/types/canonical.proto:6:8:gogoproto/gogo.proto: does not exist

@williambanfield
Copy link
Contributor

It appears we may still be missing some proto files:

proto/tendermint/consensus/wal.proto:6:8:gogoproto/gogo.proto: does not exist
proto/tendermint/privval/service.proto:5:8:tendermint/privval/types.proto: does not exist
proto/tendermint/privval/types.proto:4:8:tendermint/crypto/keys.proto: does not exist
proto/tendermint/state/types.proto:6:8:gogoproto/gogo.proto: does not exist
proto/tendermint/types/canonical.proto:6:8:gogoproto/gogo.proto: does not exist

I think that what's actually happening is that the paths changed in some places as a result of this and the generator doesn't know how to find the types anymore.

@williambanfield
Copy link
Contributor

I'm noticing a lot of references to the spec repo still in place, should we fix these up before the merge?

abci/README.md
docs/rfc/rfc-003-performance-questions.md
docs/rfc/rfc-002-ipc-ecosystem.md
docs/rfc/rfc-009-consensus-parameter-upgrades.md
docs/rfc/rfc-013-abci++.md
docs/app-dev/indexing-transactions.md
docs/architecture/adr-030-consensus-refactor.md
docs/architecture/adr-058-event-hashing.md
docs/architecture/adr-044-lite-client-with-weak-subjectivity.md
docs/architecture/adr-046-light-client-implementation.md
docs/architecture/adr-047-handling-evidence-from-light-client.md
docs/architecture/adr-053-state-sync-prototype.md
docs/architecture/adr-056-light-client-amnesia-attacks.md
docs/architecture/adr-061-p2p-refactor-scope.md
docs/architecture/adr-062-p2p-architecture.md
docs/architecture/adr-067-mempool-refactor.md
docs/architecture/adr-068-reverse-sync.md
docs/architecture/adr-071-proposer-based-timestamps.md
docs/architecture/adr-074-timeout-params.md
docs/architecture/adr-076-combine-spec-repo.md
docs/architecture/adr-077-block-retention.md
docs/architecture/adr-078-nonzero-genesis.md
docs/architecture/adr-079-ed25519-verification.md
docs/architecture/adr-080-reverse-sync.md
docs/README.md
docs/rfc/rfc-014-semantic-versioning.md
docs/tendermint-core/light-client.md
docs/tendermint-core/README.md
docs/tendermint-core/using-tendermint.md
docs/nodes/logging.md
docs/nodes/validators.md
docs/roadmap/roadmap.md
docs/app-dev/app-architecture.md
types/proposal.go
proto/tendermint/p2p/pex.proto
scripts/protopackage.sh
UPGRADING.md
internal/p2p/transport.go
README.md
CONTRIBUTING.md
spec/abci++/abci++_basic_concepts_002_draft.md
CHANGELOG.md
spec/consensus/proposer-based-timestamp/pbts-sysmodel_002_draft.md
spec/consensus/proposer-based-timestamp/tla/TendermintPBT_001_draft.tla
spec/consensus/proposer-based-timestamp/v1/pbts_001_draft.md
spec/consensus/proposer-based-timestamp/tla/TendermintPBT_002_draft.tla
spec/consensus/proposer-based-timestamp/README.md
spec/light-client/attacks/Isolation_001_draft.tla
spec/light-client/attacks/isolate-attackers_001_draft.md
spec/light-client/attacks/isolate-attackers_002_reviewed.md
spec/light-client/detection/LCDetector_003_draft.tla
spec/light-client/detection/README.md
spec/light-client/detection/detection_001_reviewed.md
spec/light-client/detection/detection_003_reviewed.md
spec/light-client/verification/verification_001_published.md
spec/light-client/verification/verification_002_draft.md
CHANGELOG_PENDING.md

@cmwaters
Copy link
Contributor Author

A lot of these reference pull requests or issues in the spec repo which I've left there. There are others which reference a commit in the spec repo which could be updated but I think it's also fine to leave it as. There's no doubt more work to be done in cleaning up the spec, I just want to get the change in quickly so as to not block people

@cmwaters
Copy link
Contributor Author

It appears we may still be missing some proto files:

Hmm I'm not sure what I changed for this to happen. gogoproto is under thirdparty/proto which is one of the root paths.

@creachadair
Copy link

It appears we may still be missing some proto files:

Hmm I'm not sure what I changed for this to happen. gogoproto is under thirdparty/proto which is one of the root paths.

I have run into something like this before—there are some subtleties about the order of processing for the various filters in the buf config.

I'm fine with merging around it, we will just have to sort it out at some point.

Copy link
Contributor

@thanethomson thanethomson left a comment

Choose a reason for hiding this comment

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

I opened #7835 to add some minor changes to links that were referring to https://github.com/tendermint/spec/blob/master. In some cases it makes more sense to just have relative links - assuming everyone's okay with relative links?

It seems as though there a quite a few dead links and some extensive cleanup's needed.

What are we going to do with all the links to docs in the rust-spec directory?

@creachadair
Copy link

creachadair commented Feb 17, 2022

I opened #7835 to add some minor changes to links that were referring to https://github.com/tendermint/spec/blob/master. In some cases it makes more sense to just have relative links - assuming everyone's okay with relative links?

It seems as though there a quite a few dead links and some extensive cleanup's needed.

What are we going to do with all the links to docs in the rust-spec directory?

The only caution about relative links is that the docs site uses a different root than the main repo. So you can either have relative links that work with the docs site or with the markdown lint checker, but not both.

See for context #7675

@cmwaters cmwaters marked this pull request as draft February 17, 2022 08:56
@cmwaters cmwaters marked this pull request as ready for review February 17, 2022 08:57
@cmwaters cmwaters merged commit 94b409e into master Feb 17, 2022
@cmwaters cmwaters deleted the callum/merge-spec branch February 17, 2022 12:51
github-merge-queue bot pushed a commit to cometbft/cometbft that referenced this pull request Jan 9, 2024
This PR cherry-picks a number of commits present in the `master` branch
of this repository.

Some of the commits are from the old `tendermint/spec` repository,
namely:

- tendermint/spec#369
- tendermint/spec#373
- tendermint/spec#375
- tendermint/spec#398
- tendermint/spec#399

This content has then been migrated to the `tendermint/tendermint`
repository, from which follow PRs were considered:

- tendermint/tendermint#7804 (**not cherry-picked**)
- tendermint/tendermint#8018
- tendermint/tendermint#8096

Most of the conflicts and the fixes needed refer to links to content,
that have been updated or referred to old repositories.

The MD files that are currently under
`spec/consensus/proposer-based-timestamp` were moved to the `v1` subdir.
Files with similar names, but the `-002` suffix were added with the new
solution, replacing the old ones.

The differences from the original content, on `master` and the content
of this PR are minimal, as one can check by running

` git diff master spec/consensus/proposer-based-timestamp/`

Finally, the entry point of the added content can be found
[here](https://github.com/cometbft/cometbft/blob/cason/pbts-spec/spec/consensus/proposer-based-timestamp/README.md).

---------

Co-authored-by: Daniel <daniel.cason@usi.ch>
Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com>
Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
Co-authored-by: Kukovec <jure.kukovec@gmail.com>
Co-authored-by: Daniel Cason <cason@gandria>
Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
mergify bot pushed a commit to cometbft/cometbft that referenced this pull request Mar 8, 2024
This PR cherry-picks a number of commits present in the `master` branch
of this repository.

Some of the commits are from the old `tendermint/spec` repository,
namely:

- tendermint/spec#369
- tendermint/spec#373
- tendermint/spec#375
- tendermint/spec#398
- tendermint/spec#399

This content has then been migrated to the `tendermint/tendermint`
repository, from which follow PRs were considered:

- tendermint/tendermint#7804 (**not cherry-picked**)
- tendermint/tendermint#8018
- tendermint/tendermint#8096

Most of the conflicts and the fixes needed refer to links to content,
that have been updated or referred to old repositories.

The MD files that are currently under
`spec/consensus/proposer-based-timestamp` were moved to the `v1` subdir.
Files with similar names, but the `-002` suffix were added with the new
solution, replacing the old ones.

The differences from the original content, on `master` and the content
of this PR are minimal, as one can check by running

` git diff master spec/consensus/proposer-based-timestamp/`

Finally, the entry point of the added content can be found
[here](https://github.com/cometbft/cometbft/blob/cason/pbts-spec/spec/consensus/proposer-based-timestamp/README.md).

---------

Co-authored-by: Daniel <daniel.cason@usi.ch>
Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com>
Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
Co-authored-by: Kukovec <jure.kukovec@gmail.com>
Co-authored-by: Daniel Cason <cason@gandria>
Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
(cherry picked from commit 84339ef)
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.