-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: update proto files for regen-ledger v5.0 and cosmos-sdk v0.46.7 #66
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are build errors but IIRC these are expected (?) so i’m approving
I need to update the amino types as well! |
dc7009a
to
08a526a
Compare
|
@@ -95,7 +95,7 @@ xdescribe('RegenApi with tendermint connection - Amino Tests', () => { | |||
|
|||
const TEST_MSG_SEND = MsgSend.fromPartial({ | |||
sender: TEST_ADDRESS, | |||
recipient: TEST_ADDRESS, | |||
recipient: TEST_BUYER_ADDRESS, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is because regen-ledger v5.0 doesn't allow sending credits to oneself regen-network/regen-ledger#1674
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good!
Will try to do some testing later.
@blushi I tested this locally through symlinking to regen-web and trying several relevant actions, but wasn’t able to test amino changes due to not having a ledger with me |
@haveanicedavid @flagrede could you test this further? |
thanks @haveanicedavid |
ref: regen-network/rnd-dev-team#1502
Note for reviewing: no need to look at all the changes in .proto and corresponding generated TS files, I guess the amino converter files are the most important to review and most importantly to test it's working (did it for most of the messages manually and made sure it's covered in our e2e tests too)
It was definitely more tedious than expected:
regen
(from v5.0) andcosmos
proto files (from v0.46.7) as well as third partycosmos_proto
using https://buf.build/cosmos/cosmos-proto/file/main:cosmos_proto/cosmos.protoRecord
and the proto messageRecord
fromcosmos.crypto.keyring.v1
which wasn’t handled yet in our forked version of ts-proto, new release: https://github.com/regen-network/ts-proto/releases/tag/v1.140.1-0 now used heresrc/generated/google/protobuf/descriptor.ts:1152:14 - error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.
Related issues: Typescript "inferred type ... exceeds maximum" error with Exact stephenh/ts-proto#449 Issue With Google protos cosmology-tech/telescope#107, settinguseExactTypes=false
in--ts_proto_opt
fixes it