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

Wrong JSON tags inside MsgSaveProfile #644

Closed
RiccardoM opened this issue Oct 7, 2021 · 4 comments
Closed

Wrong JSON tags inside MsgSaveProfile #644

RiccardoM opened this issue Oct 7, 2021 · 4 comments
Assignees
Labels
kind/bug Something isn't working x/profiles Module that allows to create and manage decentralized social profiles
Milestone

Comments

@RiccardoM
Copy link
Contributor

Bug description

From Calvin:

when signing a save profile tx, profile picture and cover picture fields are required. it can be an empty string but cant be omitted. while for name and bio, the fields can be omitted but cannot be an empty string.

This is because the profile_picture and cover_picture fields have custom attributes and do not include the omitempty clause that other fields have by default.

Expected behavior

All the fields should have the same behavior (ie. should be omittable).

@RiccardoM RiccardoM added kind/bug Something isn't working x/profiles Module that allows to create and manage decentralized social profiles labels Oct 7, 2021
@RiccardoM RiccardoM self-assigned this Oct 7, 2021
@RiccardoM RiccardoM added this to the v2.1.0 milestone Oct 7, 2021
@calvinkei
Copy link

Example of failing tx

{
    "typeUrl": "/desmos.profiles.v1beta1.MsgSaveProfile",
    "value": {
        "dtag": "testing",
        "creator": "desmos1szeh3fcef020u0uq8ynl28daznyyj7q68wgr3m"
    }
}

Example of successful tx

{
    "typeUrl": "/desmos.profiles.v1beta1.MsgSaveProfile",
    "value": {
        "dtag": "testing"
        "creator": "desmos1szeh3fcef020u0uq8ynl28daznyyj7q68wgr3m",
        "profilePicture": "",
        "coverPicture": ""
    }
}

Steps to Reproduce

  1. Create a new account on Forbole X
  2. Create a new profile for the new account
  3. Only input the dtag field and send transaction
  4. You should see an error Broadcasting transaction failed with code 4 (codespace: sdk). Log: signature verification failed; please verify account number (399), sequence (0) and chain-id (desmos-mainnet): unauthorized
  5. The transaction will succeed if you include a profilePicture and coverPicture

@RiccardoM
Copy link
Contributor Author

@calvinkei How did you generate those JSON structs? Did you use Protobuf or did you do it by hand? I'm curios where you got the profilePicture and coverPicture names specifically since they should be profile_picture and cover_picture respectively

@RiccardoM RiccardoM assigned leobragaz and unassigned RiccardoM Oct 8, 2021
@calvinkei
Copy link

@RiccardoM this is the format used in cosmjs. they replace @type by typeUrl and snake case by camel case. it will be converted to the correct format before signing. not sure why they do that though

mergify bot pushed a commit that referenced this issue Oct 18, 2021
## Description

Related to: #644.

This PR introduce ADR-009 about removing custom `jsontag`s from proto files.

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://docs.cosmos.network/v0.44/building-modules/intro.html)
- [ ] included the necessary unit and integration [tests](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
@leobragaz
Copy link
Contributor

closed by #655

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working x/profiles Module that allows to create and manage decentralized social profiles
Projects
None yet
Development

No branches or pull requests

3 participants