Migrate to gRPC + direct protobuf signing #21
Conversation
tomtau
requested review from
leejw51crypto and
yihuang
and removed request for
tomtau
generate_protos.sh
Outdated
| python -m grpc.tools.protoc --proto_path=$COSMOS/proto --proto_path=$COSMOS/third_party/proto --python_out=$OUTPUT $(find $COSMOS/proto/cosmos -iname "*.proto") --grpc_python_out=$OUTPUT --plugin=protoc-gen-grpc_python=$PLUGIN | ||
| # cosmos third-party | ||
| python -m grpc.tools.protoc --proto_path=$COSMOS/proto --proto_path=$COSMOS/third_party/proto --python_out=$OUTPUT $(find $COSMOS/third_party/proto -iname "*.proto") --grpc_python_out=$OUTPUT --plugin=protoc-gen-grpc_python=$PLUGIN |
There was a problem hiding this comment.
just need to generate what we need instead of "*.proto", see:
https://github.com/crypto-org-chain/chain-jslib/blob/dev/lib/src/cosmos/v1beta1/scripts/get-proto.sh
https://github.com/crypto-org-chain/chain-jslib/blob/dev/lib/src/cosmos/v1beta1/scripts/predefine-proto.sh
There was a problem hiding this comment.
The latest commit will let developer add path to .proto files needed in generate_protos.sh. So we only generate gRPC code that we need instead of everything.
.github/workflows/upload.yml
Outdated
| run: python3 -m pip install --user --upgrade poetry | ||
| - name: build | ||
| run: python3 setup.py sdist bdist_wheel | ||
| - name: install jfrog | ||
| run: curl -fL https://getcli.jfrog.io | sh | ||
| - name: upload to jfrog | ||
| run: | | ||
| ./jfrog rt config --url "${{ secrets.ARTIFACTORY_URL }}" --user "${{ secrets.ARTIFACTORY_USER }}" --access-token "${{ secrets.ARTIFACTORY_PASSWORD }}" --interactive=false | ||
| ./jfrog rt u --build-name=Crypto-Blockchain-PYPI --build-number=${{ github.run_id }} --flat=true "dist/*" "${{ secrets.ARTIFACTORY_NAME }}" No newline at end of file | ||
| run: poetry build | ||
| - name: release | ||
| uses: softprops/action-gh-release@v1 | ||
| with: | ||
| files: | | ||
| dist/* | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
before the security team use asked us to use jfrog to upload to pypi, peotry can do the same, right?
There was a problem hiding this comment.
For this part, I copied this GitHub workflow from pystarport. I checked with Huang Yi earlier, he said we just need to run the upload workflow and release on GitHub, then security team will pick up from there.
chainlibpy/amino/__init__.py
Outdated
| from .basic import (DEFAULT_BECH32_HRP_BASE, Coin, CommissionRates, Content, | ||
| Description, Input, Output, StdFee, SyncMode, | ||
| TimeoutHeight, VoteOptionAbstain, VoteOptionNo, | ||
| VoteOptionNoWithVeto, VoteOptionUnspecified, VoteOptionYes) | ||
| from .signdoc import StdSignDoc | ||
|
|
||
| __all__ = [ | ||
| "message", | ||
| "DEFAULT_BECH32_HRP_BASE", | ||
| "Coin", | ||
| "TimeoutHeight", | ||
| "CommissionRates", | ||
| "Content", | ||
| "Description", | ||
| "Input", | ||
| "Output", | ||
| "StdFee", | ||
| "StdSignDoc", | ||
| "SyncMode", | ||
| "VoteOptionAbstain", | ||
| "VoteOptionNo", | ||
| "VoteOptionNoWithVeto", | ||
| "VoteOptionUnspecified", | ||
| "VoteOptionYes", | ||
| ] |
There was a problem hiding this comment.
I think we should keep the amino protocol, because later it will be used in the multisign method.
There was a problem hiding this comment.
In the new commit I pushed, I recovered all files under chainlibpy/amino directory. Thank you for pointing this out :)
|
could you squash commits into 1 commit? |
| branches: | ||
| - master | ||
| tags: | ||
| - 'v*.*.*' |
There was a problem hiding this comment.
I guess, by doing such, we could control when to release by adding a tag, instead of whenever there is a MR being merged to master branch.
Squash into a single commit might reduce readability for this merge request, as the content of the merge request is quite huge, this MR is almost rewriting the entire library. |
|
looks ok |
pyproject.toml
Outdated
| @@ -1,5 +1,68 @@ | |||
| [tool.poetry] | |||
| name = "chainlibpy" | |||
| version = "2.0.0" | |||
There was a problem hiding this comment.
version need to be updated to 2.1.0 I think, because it support the grpc now.
Sorry for this huge merge request, below is a summary:
pyproject.tomlandpoetry(I am followingpystarportproject. However, there are still some files I am not sure whether to keep or remove, like.bumpversion.cfg,.coveragerc,.mypy.ini)get_balancemethod. (Note: the entire gRPC server is mocked for testing purposes, if we need more tests like this style, I could add more later.)