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

feat: clean bindings #ntrn-410 #750

Merged
merged 20 commits into from
Oct 29, 2024
Merged

feat: clean bindings #ntrn-410 #750

merged 20 commits into from
Oct 29, 2024

Conversation

NeverHappened
Copy link
Contributor

@NeverHappened NeverHappened commented Oct 14, 2024

Remove bindings and related code from neutron-sdk ,neutron-dev-contracts and integration tests
https://hadronlabs.atlassian.net/browse/NTRN-410

PR's:

  • neutron-sdk removes bindings and prototypes that are in neutron-std. Use all needed types from neutron-std;
  • neutron-dev-contracts replace bindings with grpc
  • neutron-integration-tests removed binding tests, adapter other things to new dev-contracts changed inteface
  • neutron some queries and tx's that were used in bindings were missing. Added them.
  • neutron-std regen proto types

@NeverHappened NeverHappened self-assigned this Oct 14, 2024
Copy link
Collaborator

@pr0n00gler pr0n00gler left a comment

Choose a reason for hiding this comment

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

Regenerate swagger, please

x/contractmanager/keeper/msg_server.go Outdated Show resolved Hide resolved
pr0n00gler
pr0n00gler previously approved these changes Oct 22, 2024
proto/neutron/contractmanager/tx.proto Outdated Show resolved Hide resolved
x/tokenfactory/keeper/grpc_query.go Outdated Show resolved Hide resolved
x/tokenfactory/keeper/grpc_query.go Outdated Show resolved Hide resolved
proto/osmosis/tokenfactory/v1beta1/query.proto Outdated Show resolved Hide resolved
Copy link
Contributor

@swelf19 swelf19 left a comment

Choose a reason for hiding this comment

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

Regenerate swagger pls, to reflect proto description changes

@NeverHappened
Copy link
Contributor Author

@swelf19 done

swelf19
swelf19 previously approved these changes Oct 26, 2024
proto/osmosis/tokenfactory/v1beta1/query.proto Outdated Show resolved Hide resolved
Comment on lines +44 to +45
// ResubmitFailure resubmits the failure after contract acknowledgement failed
func (k Keeper) ResubmitFailure(goCtx context.Context, req *types.MsgResubmitFailure) (*types.MsgResubmitFailureResponse, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we have ResubmitFailure and DoResubmitFailure? what's the difference between them? can we unify so there's only one Keeper's method for failures resubmission?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed DoResubmitFailure to resubmitFailure to avoid clashing. This function is keeper function and a little bit lower level dealing with store

// ResubmitFailure resubmits the failure after contract acknowledgement failed
func (k Keeper) ResubmitFailure(goCtx context.Context, req *types.MsgResubmitFailure) (*types.MsgResubmitFailureResponse, error) {
if err := req.Validate(); err != nil {
return nil, errors.Wrap(err, "failed to validate MsgUpdateParams")
Copy link
Contributor

Choose a reason for hiding this comment

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

failed to validate MsgUpdateParams?

}

if !k.wasmKeeper.HasContractInfo(ctx, sender) {
return nil, errors.Wrap(sdkerrors.ErrNotFound, "not a contract address tried to resubmit")
Copy link
Contributor

Choose a reason for hiding this comment

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

the error would look like "not a contract address tried to resubmit: not found" with codespace and code referring to sdk.

I think this way it would be more clear. wdyt about something like this?

ErrNotContractResubmission = errors.Register(ModuleName, 1104, "failures resubmission is only allowed to be called by a smart contract")

...
return nil, errors.Wrap(ErrNotContractResubmission, "sender is not a smart contract")

would be "sender is not a smart contract: failures resubmission is only allowed to be called by a smart contract" with codespace and code referring to the contractmanager module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

like it!

wasmbinding/message_plugin.go Outdated Show resolved Hide resolved
@NeverHappened
Copy link
Contributor Author

@pr0n00gler pr0n00gler merged commit 99d2060 into main Oct 29, 2024
9 checks passed
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.

4 participants