Skip to content
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

update interface proto annotations #1156

Merged
merged 3 commits into from
Jan 30, 2023

Conversation

pyramation
Copy link
Contributor

closes #1155

@pyramation pyramation requested a review from alpe as a code owner January 14, 2023 03:42
@@ -85,14 +85,14 @@ message CombinedLimit {
// message.
// Since: wasmd 0.30
message AllowAllMessagesFilter {
option (cosmos_proto.implements_interface) = "ContractAuthzFilterX";
option (cosmos_proto.implements_interface) = "cosmwasm.wasm.ContractAuthzFilterX";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. registered interface here
  2. interface defined here

would this make it cosmwasm.wasm.ContractAuthzFilterX?

@@ -57,7 +57,7 @@ message MaxCallsLimit {
// MaxFundsLimit defines the maximal amounts that can be sent to the contract.
// Since: wasmd 0.30
message MaxFundsLimit {
option (cosmos_proto.implements_interface) = "ContractAuthzLimitX";
option (cosmos_proto.implements_interface) = "cosmwasm.wasm.ContractAuthzLimitX";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. registered interface here
  2. interface defined here

would this make it cosmwasm.wasm.ContractAuthzLimitX?

Copy link
Contributor

Choose a reason for hiding this comment

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

You should have the version number here, too: cosmwasm.wasm.v1.ContractAuthzLimitX

@@ -90,7 +90,7 @@ message ContractInfo {
// Extension is an extension point to store custom metadata within the
// persistence model.
google.protobuf.Any extension = 7
[ (cosmos_proto.accepts_interface) = "ContractInfoExtension" ];
[ (cosmos_proto.accepts_interface) = "cosmwasm.wasm.ContractInfoExtension" ];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@alpe alpe left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this PR. Too bad that I did all of this in #1149 already for sdk 0.47 as well as adding the signer annotation. Merging your PR will create merge conflicts for the sdk 0.47 branch.

Are the annotations something you need already in the next wasmd release or can this wait?
if yes, please add the version to the cosmwasm interface names and run make proto-all

@@ -57,7 +57,7 @@ message MaxCallsLimit {
// MaxFundsLimit defines the maximal amounts that can be sent to the contract.
// Since: wasmd 0.30
message MaxFundsLimit {
option (cosmos_proto.implements_interface) = "ContractAuthzLimitX";
option (cosmos_proto.implements_interface) = "cosmwasm.wasm.ContractAuthzLimitX";
Copy link
Contributor

Choose a reason for hiding this comment

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

You should have the version number here, too: cosmwasm.wasm.v1.ContractAuthzLimitX

@pyramation
Copy link
Contributor Author

Thanks a lot for this PR. Too bad that I did all of this in #1149 already for sdk 0.47 as well as adding the signer annotation. Merging your PR will create merge conflicts for the sdk 0.47 branch.

Are the annotations something you need already in the next wasmd release or can this wait? if yes, please add the version to the cosmwasm interface names and run make proto-all

Ideally we can get these ASAP, I'd be down to help make another PR to avoid the conflicts. Let me know the best approach!

@alpe
Copy link
Contributor

alpe commented Jan 24, 2023

Thanks for the updates!
Please run make proto-all to format the proto files and have updated Go files.
When done, I can include this for the v0.31.0 release.

Not sure how to address the merge issues with #1149 afterwards. I may simply revert this PR so let's make sure #1149 contains all the proto options that you need, too.

@alpe
Copy link
Contributor

alpe commented Jan 26, 2023

@pyramation please see my last comment, to get this into v0.31.0

@pyramation
Copy link
Contributor Author

@pyramation please see my last comment, to get this into v0.31.0

should I maybe do this again, but branch off of #1149 ?

@alpe
Copy link
Contributor

alpe commented Jan 27, 2023

@pyramation just run make proto-all on this branch to have proper protobuf and generated Go files. This is only relevant if you need them for the v0.31 release. If not, let's close this PR without merging

I would like to get #1149 (which includes all the protobuf options of this PR already) into main as soon as the release is cut.

@alpe
Copy link
Contributor

alpe commented Jan 27, 2023

fyi: I have tagged a rc0 release. There is still time on Monday to get this into the final one

@sonarcloud
Copy link

sonarcloud bot commented Jan 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@pyramation
Copy link
Contributor Author

great! I just ran the make proto-all and pushed :)

@alpe alpe merged commit 327bb06 into CosmWasm:main Jan 30, 2023
@alpe
Copy link
Contributor

alpe commented Jan 30, 2023

👍 Thanks for the updates! 💯

alpe added a commit that referenced this pull request Mar 14, 2023
* main: (36 commits)
  Add changelog for v0.31.0 (#1188)
  Upgrade to wasmvm 1.2.1 (#1245)
  Bump bufbuild/buf-setup-action from 1.15.0 to 1.15.1
  Make `CaptureIbcEvents` in ibctesting public.
  Fix client checksum verification (#1234)
  Test rust panic for regression
  Return IBC packet sequence number (#1225)
  Rename windows client binary
  Configure sonarcloud analysis
  Bump github.com/cosmos/gogoproto from 1.4.3 to 1.4.6
  Add Windows client support (#1197)
  Bump github.com/cosmos/cosmos-proto from 1.0.0-beta.1 to 1.0.0-beta.2
  Bump bufbuild/buf-setup-action from 1.14.0 to 1.15.0
  Bump SDK version to v0.45.14
  Bump bufbuild/buf-setup-action from 1.13.1 to 1.14.0 (#1200)
  Bump github.com/stretchr/testify from 1.8.1 to 1.8.2 (#1211)
  test: add unit test
  fix: stargate querier does not reset the state
  list-contract-by-code bugfix
  update interface proto annotations (#1156)
  ...
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.

clean up {accept,implement}_interface in protos
2 participants