Skip to content
This repository has been archived by the owner on Feb 23, 2022. It is now read-only.

update proto generation and testing pipelines #358

Merged
merged 22 commits into from
Nov 16, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions .clang-format
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
Language: Proto
BasedOnStyle: Google
IndentWidth: 2
ColumnLimit: 0
AlignConsecutiveAssignments: true
AlignConsecutiveDeclarations: true
SpacesInSquareBrackets: true
ReflowComments: true
SortIncludes: true
SortUsingDeclarations: true
23 changes: 23 additions & 0 deletions .github/workflows/proto-check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Proto Check
# Protobuf runs buf (https://buf.build/) lint and check-breakage
# This workflow is only run when a .proto file has been modified
williambanfield marked this conversation as resolved.
Show resolved Hide resolved
on:
workflow_dispatch: # allow running workflow manually
pull_request:
paths:
- "./proto"
jobs:
proto-lint:
runs-on: ubuntu-latest
timeout-minutes: 4
steps:
- uses: actions/checkout@v2.4.0
- name: lint
run: make proto-lint
proto-breakage:
runs-on: ubuntu-latest
timeout-minutes: 4
steps:
- uses: actions/checkout@v2.4.0
- name: check-breakage
run: make proto-check-breaking-ci
51 changes: 51 additions & 0 deletions .github/workflows/proto-dockerfile.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
name: Build & Push TM Proto Builder
williambanfield marked this conversation as resolved.
Show resolved Hide resolved
on:
pull_request:
paths:
- "proto/*"
push:
branches:
- master
paths:
- "proto/*"
schedule:
# run this job once a month to recieve any go or buf updates
- cron: "* * 1 * *"

jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2.4.0
- name: Prepare
id: prep
run: |
DOCKER_IMAGE=tendermintdev/docker-build-proto
VERSION=noop
if [[ $GITHUB_REF == refs/tags/* ]]; then
VERSION=${GITHUB_REF#refs/tags/}
elif [[ $GITHUB_REF == refs/heads/* ]]; then
VERSION=$(echo ${GITHUB_REF#refs/heads/} | sed -r 's#/+#-#g')
if [ "${{ github.event.repository.default_branch }}" = "$VERSION" ]; then
VERSION=latest
fi
fi
TAGS="${DOCKER_IMAGE}:${VERSION}"
echo ::set-output name=tags::${TAGS}

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v1.6.0

- name: Login to DockerHub
uses: docker/login-action@v1.10.0
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}

- name: Publish to Docker Hub
uses: docker/build-push-action@v2.7.0
with:
context: ./proto
file: ./proto/Dockerfile
push: ${{ github.event_name != 'pull_request' }}
tags: ${{ steps.prep.outputs.tags }}
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
*.gz
*.dvi
.idea
*.pb.go
31 changes: 31 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
DOCKER_PROTO_BUILDER := docker run -v $(shell pwd):/workspace --workdir /workspace tendermintdev/docker-build-proto
HTTPS_GIT := https://github.com/tendermint/spec.git

###############################################################################
### Protobuf ###
###############################################################################

proto-all: proto-lint proto-check-breaking
.PHONY: proto-all

proto-gen:
@echo "Generating Protobuf files"
@$(DOCKER_PROTO_BUILDER) buf generate --template=./proto/buf.gen.yaml --config ./proto/buf.yaml
.PHONY: proto-gen

proto-lint:
@$(DOCKER_PROTO_BUILDER) buf lint --error-format=json --config ./proto/buf.yaml
.PHONY: proto-lint

proto-format:
@echo "Formatting Protobuf files"
@$(DOCKER_PROTO_BUILDER) find . -name '*.proto' -path "./proto/*" -exec clang-format -i {} \;
.PHONY: proto-format

proto-check-breaking:
@$(DOCKER_PROTO_BUILDER) buf breaking --against .git --config ./proto/buf.yaml
.PHONY: proto-check-breaking

proto-check-breaking-ci:
@$(DOCKER_PROTO_BUILDER) buf breaking --against $(HTTPS_GIT) --config ./proto/buf.yaml
.PHONY: proto-check-breaking-ci
24 changes: 24 additions & 0 deletions proto/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# This Dockerfile defines a toolbox for use in linting, formatting, and compiling the Tendermint protos.

williambanfield marked this conversation as resolved.
Show resolved Hide resolved
FROM bufbuild/buf:latest as buf

FROM alpine:3.14

# Install a commonly used set of programs for use with our protos.
# clang-extra-tools is included here because it provides clang-format, the program used for formatting the protos.
williambanfield marked this conversation as resolved.
Show resolved Hide resolved
RUN echo 'http://dl-cdn.alpinelinux.org/alpine/edge/testing' >> /etc/apk/repositories && \
williambanfield marked this conversation as resolved.
Show resolved Hide resolved
apk add --update --no-cache build-base clang-extra-tools curl git go && \
rm -rf /var/cache/apk/*

ENV GOLANG_PROTOBUF_VERSION=1.3.1 \
GOGO_PROTOBUF_VERSION=1.3.2

# Retrieve the go protoc programs and copy them into the PATH
RUN GO111MODULE=on go get \
github.com/golang/protobuf/protoc-gen-go@v${GOLANG_PROTOBUF_VERSION} \
github.com/gogo/protobuf/protoc-gen-gogo@v${GOGO_PROTOBUF_VERSION} \
github.com/gogo/protobuf/protoc-gen-gogofaster@v${GOGO_PROTOBUF_VERSION} && \
mv $(go env | grep GOPATH | sed 's/GOPATH="\(.*\)"/\1/g')/bin/* /usr/local/bin/
williambanfield marked this conversation as resolved.
Show resolved Hide resolved

# Copy the 'buf' program out of the buildbuf/buf container.
COPY --from=buf /usr/local/bin/* /usr/local/bin/
13 changes: 13 additions & 0 deletions proto/buf.gen.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# The version of the generation template.
# Required.
# The only currently-valid value is v1beta1.
version: v1beta1

# The plugins to run.
plugins:
# The name of the plugin.
- name: gogofaster
# The the relative output directory.
out: proto
# Any options to provide to the plugin.
opt: Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/duration.proto=github.com/golang/protobuf/ptypes/duration,plugins=grpc,paths=source_relative
16 changes: 16 additions & 0 deletions proto/buf.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
version: v1beta1

build:
roots:
- proto
- third_party/proto
lint:
use:
- BASIC
- FILE_LOWER_SNAKE_CASE
- UNARY_RPC
ignore:
- gogoproto
breaking:
use:
- FILE
49 changes: 33 additions & 16 deletions proto/tendermint/abci/types.proto
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
syntax = "proto3";
package tendermint.abci;

option go_package = "github.com/tendermint/tendermint/abci/types";
Copy link
Contributor

Choose a reason for hiding this comment

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

what the new process here? generate in spec then move over?

I think for go this is required? (I can't remember exactly)

Copy link
Contributor

Choose a reason for hiding this comment

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

when this is imported into Tendermint, will it not cause package issues?

package tendermint_abci

Copy link
Contributor

Choose a reason for hiding this comment

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

whoops, never mind, I read the other PR now.

The only downside of doing it this way is, users of our photo files will need to append option go_package as well, this could cause confusion. Before it was plug and play.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, this is really a downside of hosting the protos in the spec repo in general. If users were to try to generate these protos themselves using the old option, they would have created protos with import paths that do not match their local module name. As a result of this, they would have two options, both of which are not ideal.

Option 1. Rewrite the import paths locally, either before or after proto generation so that the paths match the local module. This is effectively what we are changing Tendermint to do.
Option 2. Shove the protos into the vendor directory, where packages are allowed to have a name that does not match the module name, but is also a directory that go programmers frequently cede to the go mod tooling to manage.

Neither option is great, and I think we can all agree that using the module name github.com/tendermint/tendermint in this package is strictly incorrect. If this solution causes serious headache for users who are attempting to vendor these protos into their local vendor directory, we can reconsider, but it also seems odd to me that a user would do that and not simply import the generated protos that exist within the Tendermint repository instead since they are effectively equivalent.

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue is that when the user imports the proto file into there proto file and generate their files the go import will be set to something else that isn't actually importing a file. This is how it is currently done for all go projects.

It's fine if we break this, wanted to make note of it mainly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, do you know of people doing this?


// For more information on gogo.proto, see:
// https://github.com/gogo/protobuf/blob/master/extensions.md
import "tendermint/crypto/proof.proto";
Expand Down Expand Up @@ -52,7 +50,8 @@ message RequestInfo {
}

message RequestInitChain {
google.protobuf.Timestamp time = 1 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true];
google.protobuf.Timestamp time = 1
[(gogoproto.nullable) = false, (gogoproto.stdtime) = true];
string chain_id = 2;
tendermint.types.ConsensusParams consensus_params = 3;
repeated ValidatorUpdate validators = 4 [(gogoproto.nullable) = false];
Expand Down Expand Up @@ -182,7 +181,10 @@ message ResponseQuery {
}

message ResponseBeginBlock {
repeated Event events = 1 [(gogoproto.nullable) = false, (gogoproto.jsontag) = "events,omitempty"];
repeated Event events = 1 [
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "events,omitempty"
];
}

message ResponseCheckTx {
Expand All @@ -192,10 +194,13 @@ message ResponseCheckTx {
string info = 4; // nondeterministic
int64 gas_wanted = 5 [json_name = "gas_wanted"];
int64 gas_used = 6 [json_name = "gas_used"];
repeated Event events = 7 [(gogoproto.nullable) = false, (gogoproto.jsontag) = "events,omitempty"];
string codespace = 8;
string sender = 9;
int64 priority = 10;
repeated Event events = 7 [
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "events,omitempty"
];
string codespace = 8;
string sender = 9;
int64 priority = 10;

// mempool_error is set by Tendermint.
// ABCI applications creating a ResponseCheckTX should not set mempool_error.
Expand All @@ -209,15 +214,21 @@ message ResponseDeliverTx {
string info = 4; // nondeterministic
int64 gas_wanted = 5 [json_name = "gas_wanted"];
int64 gas_used = 6 [json_name = "gas_used"];
repeated Event events = 7
[(gogoproto.nullable) = false, (gogoproto.jsontag) = "events,omitempty"]; // nondeterministic
repeated Event events = 7 [
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "events,omitempty"
]; // nondeterministic
string codespace = 8;
}

message ResponseEndBlock {
repeated ValidatorUpdate validator_updates = 1 [(gogoproto.nullable) = false];
repeated ValidatorUpdate validator_updates = 1
[(gogoproto.nullable) = false];
tendermint.types.ConsensusParams consensus_param_updates = 2;
repeated Event events = 3 [(gogoproto.nullable) = false, (gogoproto.jsontag) = "events,omitempty"];
repeated Event events = 3 [
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "events,omitempty"
];
}

message ResponseCommit {
Expand Down Expand Up @@ -275,7 +286,10 @@ message LastCommitInfo {
// Later, transactions may be queried using these events.
message Event {
string type = 1;
repeated EventAttribute attributes = 2 [(gogoproto.nullable) = false, (gogoproto.jsontag) = "attributes,omitempty"];
repeated EventAttribute attributes = 2 [
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "attributes,omitempty"
];
}

// EventAttribute is a single key-value pair, associated with an event.
Expand Down Expand Up @@ -330,7 +344,8 @@ message Evidence {
// The height when the offense occurred
int64 height = 3;
// The corresponding time where the offense occurred
google.protobuf.Timestamp time = 4 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true];
google.protobuf.Timestamp time = 4
[(gogoproto.nullable) = false, (gogoproto.stdtime) = true];
// Total voting power of the validator set in case the ABCI application does
// not store historical validators.
// https://github.com/tendermint/tendermint/issues/4581
Expand Down Expand Up @@ -364,6 +379,8 @@ service ABCIApplication {
rpc EndBlock(RequestEndBlock) returns (ResponseEndBlock);
rpc ListSnapshots(RequestListSnapshots) returns (ResponseListSnapshots);
rpc OfferSnapshot(RequestOfferSnapshot) returns (ResponseOfferSnapshot);
rpc LoadSnapshotChunk(RequestLoadSnapshotChunk) returns (ResponseLoadSnapshotChunk);
rpc ApplySnapshotChunk(RequestApplySnapshotChunk) returns (ResponseApplySnapshotChunk);
rpc LoadSnapshotChunk(RequestLoadSnapshotChunk)
returns (ResponseLoadSnapshotChunk);
rpc ApplySnapshotChunk(RequestApplySnapshotChunk)
returns (ResponseApplySnapshotChunk);
}
8 changes: 3 additions & 5 deletions proto/tendermint/blocksync/types.proto
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
syntax = "proto3";
package tendermint.blocksync;

option go_package = "github.com/tendermint/tendermint/proto/tendermint/blocksync";

import "tendermint/types/block.proto";

// BlockRequest requests a block for a specific height
message BlockRequest {
int64 height = 1;
}

// NoBlockResponse informs the node that the peer does not have block at the requested height
// NoBlockResponse informs the node that the peer does not have block at the
// requested height
message NoBlockResponse {
int64 height = 1;
}
Expand All @@ -21,8 +20,7 @@ message BlockResponse {
}

// StatusRequest requests the status of a peer.
message StatusRequest {
}
message StatusRequest {}

// StatusResponse is a peer response to inform their status.
message StatusResponse {
Expand Down
29 changes: 17 additions & 12 deletions proto/tendermint/consensus/types.proto
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
syntax = "proto3";
package tendermint.consensus;

option go_package = "github.com/tendermint/tendermint/proto/tendermint/consensus";

import "gogoproto/gogo.proto";
import "tendermint/types/types.proto";
import "tendermint/libs/bits/types.proto";
Expand All @@ -17,15 +15,18 @@ message NewRoundStep {
int32 last_commit_round = 5;
}

// NewValidBlock is sent when a validator observes a valid block B in some round r,
//i.e., there is a Proposal for block B and 2/3+ prevotes for the block B in the round r.
// NewValidBlock is sent when a validator observes a valid block B in some round
// r,
// i.e., there is a Proposal for block B and 2/3+ prevotes for the block B in
// the round r.
// In case the block is also committed, then IsCommit flag is set to true.
message NewValidBlock {
int64 height = 1;
int32 round = 2;
tendermint.types.PartSetHeader block_part_set_header = 3 [(gogoproto.nullable) = false];
tendermint.libs.bits.BitArray block_parts = 4;
bool is_commit = 5;
tendermint.types.PartSetHeader block_part_set_header = 3
[(gogoproto.nullable) = false];
tendermint.libs.bits.BitArray block_parts = 4;
bool is_commit = 5;
}

// Proposal is sent when a new block is proposed.
Expand All @@ -37,7 +38,8 @@ message Proposal {
message ProposalPOL {
int64 height = 1;
int32 proposal_pol_round = 2;
tendermint.libs.bits.BitArray proposal_pol = 3 [(gogoproto.nullable) = false];
tendermint.libs.bits.BitArray proposal_pol = 3
[(gogoproto.nullable) = false];
}

// BlockPart is sent when gossipping a piece of the proposed block.
Expand Down Expand Up @@ -65,16 +67,19 @@ message VoteSetMaj23 {
int64 height = 1;
int32 round = 2;
tendermint.types.SignedMsgType type = 3;
tendermint.types.BlockID block_id = 4 [(gogoproto.customname) = "BlockID", (gogoproto.nullable) = false];
tendermint.types.BlockID block_id = 4
[(gogoproto.customname) = "BlockID", (gogoproto.nullable) = false];
}

// VoteSetBits is sent to communicate the bit-array of votes seen for the BlockID.
// VoteSetBits is sent to communicate the bit-array of votes seen for the
// BlockID.
message VoteSetBits {
int64 height = 1;
int32 round = 2;
tendermint.types.SignedMsgType type = 3;
tendermint.types.BlockID block_id = 4 [(gogoproto.customname) = "BlockID", (gogoproto.nullable) = false];
tendermint.libs.bits.BitArray votes = 5 [(gogoproto.nullable) = false];
tendermint.types.BlockID block_id = 4
[(gogoproto.customname) = "BlockID", (gogoproto.nullable) = false];
tendermint.libs.bits.BitArray votes = 5 [(gogoproto.nullable) = false];
}

message Message {
Expand Down
2 changes: 0 additions & 2 deletions proto/tendermint/crypto/keys.proto
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
syntax = "proto3";
package tendermint.crypto;

option go_package = "github.com/tendermint/tendermint/proto/tendermint/crypto";

import "gogoproto/gogo.proto";

// PublicKey defines the keys available for use with Tendermint Validators
Expand Down