cosmos v0.44 support #28

ctrl-Felix · 2021-12-24T12:21:17Z

add cosmos v0.44 support

for more information, see https://pre-commit.ci

czarcas7ic

This fixed pushing txs to the new endpoint, tysm for this

czarcas7ic · 2021-12-25T21:56:35Z

But also to note, I had to change one line under get_pushable command

        return json.dumps(
           # {"jsonrpc": "2.0", "id": 1, "method": self._sync_mode, "params": {"tx": tx_b64}}
           { "tx_bytes": tx_b64, "mode": "BROADCAST_MODE_SYNC"}

ctrl-Felix · 2021-12-25T23:12:42Z

I had to change one line under get_pushable command

Wasn't it working like it was there? Because I submitted transactions like this

hukkin

Thanks a lot!

I'm not yet able to have a good look unfortunately but here's my first drive-by review.

A couple questions:

where did you get the .proto files?
are all the .proto files strictly needed? Is this the smallest set that works?
what command did you run to generate .py files from the .proto files?

hukkin · 2021-12-25T23:12:17Z

README.md

+rpc_url = "https://rpc.cosmos.network/"
+
+# Method 1
+r = tx.broadcast(url=rpc_url)


I don't really want this API or the requests dependency. Cosmospy is not a networking library.

hukkin · 2021-12-25T23:12:43Z

README.md

+import requests
+
+pushable_tx = tx.get_pushable()
+r = requests.post(rpc_url, data=pushable_tx)


Let's stick to httpx here.

hukkin · 2021-12-25T23:14:07Z

pyproject.toml

@@ -27,6 +27,10 @@ dependencies = [
    "mnemonic >=0.19",
    "hdwallets >=0.1.0",
    "typing-extensions >=3.7.4; python_version < '3.8'",
+    "requests==2.26.0",
+    "protobuf >= 3.19.1",
+    "google >= 3.0.0",


where is google dependency used? Do we really need it?

hukkin · 2021-12-25T23:15:18Z

src/cosmospy/_transaction.py


+from cosmospy._typing import SyncMode


typing module should remain public API. (no underscore prefix)

hukkin · 2021-12-25T23:18:48Z

src/cosmospy/_transaction.py

+
+    def _get_pubkey(self):
+        pubkey_bytes = privkey_to_pubkey(self._privkey)
+        _pubkey = pubkey.PubKey()


There shouldn't be a reason to use private variables inside the function scope (applies to other methods too)

hukkin · 2021-12-25T23:19:30Z

src/cosmospy/_typing.py

@@ -1,14 +1,15 @@
 import sys

+print(sys.version_info)


This should be removed

hukkin · 2021-12-25T23:21:46Z

src/cosmospy/_typing.py

 if sys.version_info < (3, 8):
    from typing_extensions import Literal, TypedDict
 else:
    from typing import Literal, TypedDict


-# Valid transaction broadcast modes for the `POST /txs` endpoint of the
+# Valid transaction broadcast modes for the TX endpoint of the


POST /cosmos/tx/v1beta1/txs endpoint

hukkin · 2021-12-25T23:30:09Z

I had to change one line under get_pushable command

Wasn't it working like it was there? Because I submitted transactions like this

I'm fairly sure @czarcas7ic is correct here. Based on swagger docs of the POST /cosmos/tx/v1beta1/txs endpoint.

czarcas7ic · 2021-12-26T01:18:22Z

I had to change one line under get_pushable command

Wasn't it working like it was there? Because I submitted transactions like this

Also see https://docs.cosmos.network/master/core/proto-docs.html#cosmos-tx-v1beta1-service-proto

ctrl-Felix · 2021-12-26T09:56:35Z

I had to change one line under get_pushable command

Wasn't it working like it was there? Because I submitted transactions like this

I'm fairly sure @czarcas7ic is correct here. Based on swagger docs of the POST /cosmos/tx/v1beta1/txs endpoint.

I think it just depends if you use the API or RPC. If you want to use it with the RPC endpoint it works. That's why I changed the endpoint to https://rpc.cosmos.network and not the api endpoint.

If you want to submit it to the API you need the change @czarcas7ic made.

It would be interesting to know if there is a performance difference

ctrl-Felix · 2021-12-26T10:04:19Z

Thanks a lot!

I'm not yet able to have a good look unfortunately but here's my first drive-by review.

A couple questions:
* where did you get the `.proto` files?

* are all the `.proto` files strictly needed? Is this the smallest set that works?

* what command did you run to generate `.py` files from the `.proto` files?

I took the proto files from cyberpy but I think those are the sames they also used in their js library. I will check if they are all needed. This version is just a really fast adaption because I wanted to provide a fix for everyone using cosmospy (also for me I wasn't able to vroadcast transactions in python anymore)

I took the generated python files also from cybai but you can generate those files with tools like https://github.com/dropbox/mypy-protobuf, I will setup a proper way to generate them also in case of changements.

Regarding the google dep it is needed as everything relies on google.protobuf and if google or protobuf aren't installed you get the error ModuleNotFoundError: No module named 'google.protobuf'

Also the current tests will need to be recreated to work with the new update.

hukkin · 2021-12-26T11:11:47Z

I took the proto files from cyberpy but I think those are the sames they also used in their js library. I will check if they are all needed.

Thanks. Would be good to know what is cyberpys source for these files instead of blindly copying them.

Regarding the google dep it is needed as everything relies on google.protobuf and if google or protobuf aren't installed you get the error ModuleNotFoundError: No module named 'google.protobuf'

The google.protobuf namespace should be available solely from protobuf package. See https://github.com/protocolbuffers/protobuf/tree/master/python where the source code is under google/protobuf/.

ctrl-Felix · 2022-01-15T22:58:30Z

Okay, I have some news regarding the proto files

The proto files are coming from cosmos (ex: https://github.com/cosmos/cosmos-sdk/blob/master/proto/cosmos/base/v1beta1/coin.proto => https://github.com/ctrl-Felix/cosmospy/blob/sdkv0.44/src/cosmospy/proto/coin.proto)

There are then cli utilities to migrate those protos to python code.

I saw some proto files have been updated on cosmos or have more content. I will update them on my repo too.

ctrl-Felix · 2022-01-15T23:04:52Z

The google.protobuf namespace should be available solely from protobuf package. See https://github.com/protocolbuffers/protobuf/tree/master/python where the source code is under google/protobuf/.

I am confused too but it needs both. Check out this thread: https://stackoverflow.com/questions/38680593/importerror-no-module-named-google-protobuf

There is no explanation to this

hukkin · 2022-01-15T23:09:25Z

I am confused too but it needs both. Check out this thread: https://stackoverflow.com/questions/38680593/importerror-no-module-named-google-protobuf

Did you try removing the google dependency? We don't do import google, the autogenerated files do from google.protobuf import *. I don't think we need the dependency.

hukkin · 2022-01-15T23:13:04Z

Okay, I have some news regarding the proto files

I think this may be the best source for the proto files https://buf.build/cosmos/cosmos-sdk/tree

What we need is

the minimal set of proto files that we need for cosmospy to work (downloaded from the resource above)
the command to autogenerate Python

I cannot accept a PR with autogenerated files that I can not verify/reproduce myself.

ctrl-Felix · 2022-01-15T23:17:38Z

I will close the pr anyways and redo it with a new version where the protos are generated from the official ones. This PR is probbaly more a dirty quick fix.

ctrl-Felix · 2022-01-15T23:19:09Z

I am confused too but it needs both. Check out this thread: https://stackoverflow.com/questions/38680593/importerror-no-module-named-google-protobuf

Did you try removing the google dependency? We don't do import google, the autogenerated files do from google.protobuf import *. I don't think we need the dependency.

ah yeah, you're right. Google is the one which isn't needed.

hukkin · 2022-01-15T23:28:31Z

BTW, here's the docs for compiling Python from .proto https://developers.google.com/protocol-buffers/docs/pythontutorial#compiling-your-protocol-buffers

Here's a link to download the (currently) latest protoc binaries https://github.com/protocolbuffers/protobuf/releases/tag/v3.19.3

ctrl-Felix · 2022-02-13T18:25:35Z

I have some problems with converting the proto files to python files. AS Example:
Coin.proto looks like this on cyberpy https://github.com/SaveTheAles/cyberpy/blob/master/cyberpy/interfaces/coin_pb2.py
My converted version looks like this https://github.com/ctrl-Felix/cosmospy/blob/master/src/cosmospy/generated/coin_pb2.py

ctrl-Felix and others added 2 commits December 24, 2021 13:20

cosmos v0.44 support

f8e12ee

add cosmos v0.44 support

[pre-commit.ci] auto fixes from pre-commit.com hooks

e0079f7

for more information, see https://pre-commit.ci

ctrl-Felix mentioned this pull request Dec 24, 2021

REST endpoint migration #24

Open

ctrl-Felix and others added 4 commits December 24, 2021 13:24

add dependencies

baaec43

Merge remote-tracking branch 'origin/sdkv0.44' into sdkv0.44

7762922

fix hrp for transfers & Update README

d12341f

[pre-commit.ci] auto fixes from pre-commit.com hooks

f21794b

for more information, see https://pre-commit.ci

czarcas7ic reviewed Dec 25, 2021

View reviewed changes

hukkin suggested changes Dec 25, 2021

View reviewed changes

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cosmos v0.44 support #28

cosmos v0.44 support #28

ctrl-Felix commented Dec 24, 2021

czarcas7ic left a comment

czarcas7ic commented Dec 25, 2021

ctrl-Felix commented Dec 25, 2021

hukkin left a comment

hukkin Dec 25, 2021

hukkin Dec 25, 2021

hukkin Dec 25, 2021

hukkin Dec 25, 2021

hukkin Dec 25, 2021

hukkin Dec 25, 2021

hukkin Dec 25, 2021

hukkin commented Dec 25, 2021

czarcas7ic commented Dec 26, 2021

ctrl-Felix commented Dec 26, 2021 •

edited

Loading

ctrl-Felix commented Dec 26, 2021 •

edited

Loading

hukkin commented Dec 26, 2021

ctrl-Felix commented Jan 15, 2022

ctrl-Felix commented Jan 15, 2022

hukkin commented Jan 15, 2022

hukkin commented Jan 15, 2022 •

edited

Loading

ctrl-Felix commented Jan 15, 2022

ctrl-Felix commented Jan 15, 2022

hukkin commented Jan 15, 2022

ctrl-Felix commented Feb 13, 2022

cosmos v0.44 support #28

cosmos v0.44 support #28

Conversation

ctrl-Felix commented Dec 24, 2021

czarcas7ic left a comment

Choose a reason for hiding this comment

czarcas7ic commented Dec 25, 2021

ctrl-Felix commented Dec 25, 2021

hukkin left a comment

Choose a reason for hiding this comment

hukkin Dec 25, 2021

Choose a reason for hiding this comment

hukkin Dec 25, 2021

Choose a reason for hiding this comment

hukkin Dec 25, 2021

Choose a reason for hiding this comment

hukkin Dec 25, 2021

Choose a reason for hiding this comment

hukkin Dec 25, 2021

Choose a reason for hiding this comment

hukkin Dec 25, 2021

Choose a reason for hiding this comment

hukkin Dec 25, 2021

Choose a reason for hiding this comment

hukkin commented Dec 25, 2021

czarcas7ic commented Dec 26, 2021

ctrl-Felix commented Dec 26, 2021 • edited Loading

ctrl-Felix commented Dec 26, 2021 • edited Loading

hukkin commented Dec 26, 2021

ctrl-Felix commented Jan 15, 2022

ctrl-Felix commented Jan 15, 2022

hukkin commented Jan 15, 2022

hukkin commented Jan 15, 2022 • edited Loading

ctrl-Felix commented Jan 15, 2022

ctrl-Felix commented Jan 15, 2022

hukkin commented Jan 15, 2022

ctrl-Felix commented Feb 13, 2022

ctrl-Felix commented Dec 26, 2021 •

edited

Loading

ctrl-Felix commented Dec 26, 2021 •

edited

Loading

hukkin commented Jan 15, 2022 •

edited

Loading