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: update x/collection to use Finschia/cosmos-sdk #1208

Merged
merged 42 commits into from
Mar 5, 2024

Conversation

jaeseung-bae
Copy link
Contributor

@jaeseung-bae jaeseung-bae commented Jan 15, 2024

Description

closes: #1196

  • Remove useless features
    • codec init (it's injected by cosmos/runtime now)
    • msg's GetSigners(), Type(), Route(), GetSignBytes() -> sdk.Msg doesn't require those method anymore.
    • remove FT features and its stored data
  • Migrate x/token's Class feature/data into x/collection module
  • Change some error codespace to maintain easily
    • ErrInvalidPermission ( [link, 2] -> [collection, 101] )
    • ErrInvalidContractID ( [contract, 2] -> [collection, 102] )
    • ErrContractNotExist ( [contract, 3] -> [collection, 103] )
  • Apply v0.50.x to x/collection
  • CLITestSuite verifies CLI processes before broadcasting.
  • E2ETestSuite for gRPC, Tx

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Checklist:

  • I followed the contributing guidelines and code of conduct.
  • I have added a relevant changelog to CHANGELOG.md
  • I have added tests to cover my changes.
  • I have updated the documentation accordingly.
  • I have updated API documentation client/docs/swagger-ui/swagger.yaml

@jaeseung-bae jaeseung-bae changed the base branch from main to bumpup50 January 15, 2024 04:48
@jaeseung-bae jaeseung-bae self-assigned this Jan 22, 2024
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

gosec found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@jaeseung-bae jaeseung-bae force-pushed the bumpup50-collection branch 2 times, most recently from e0ffc7b to 0e7d0a9 Compare January 25, 2024 07:33
@jaeseung-bae jaeseung-bae changed the title Bumpup50 collection feat: update x/collection to use Finschia/cosmos-sdk Jan 25, 2024
@tkxkd0159 tkxkd0159 self-assigned this Feb 7, 2024
@jaeseung-bae jaeseung-bae removed their assignment Feb 7, 2024
Copy link
Collaborator

@0Tech 0Tech left a comment

Choose a reason for hiding this comment

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

We'd better to apply address codec to this module. It will involve removing usages of AccAddressFromBech32() and String() on the addresses.

scripts/mockgen.sh Outdated Show resolved Hide resolved
tests/e2e/collection/grpc.go Outdated Show resolved Hide resolved
tests/e2e/collection/grpc.go Outdated Show resolved Hide resolved
tests/e2e/collection/grpc.go Show resolved Hide resolved
x/collection/keeper/migrations.go Show resolved Hide resolved
x/collection/keeper/migrations/v3/store.go Show resolved Hide resolved
x/collection/keeper/send.go Outdated Show resolved Hide resolved
x/collection/keeper/supply.go Outdated Show resolved Hide resolved
x/collection/keeper/supply.go Outdated Show resolved Hide resolved
x/collection/msgs.go Outdated Show resolved Hide resolved
proto/lbm/collection/v1/collection.proto Outdated Show resolved Hide resolved
proto/lbm/collection/v1/genesis.proto Outdated Show resolved Hide resolved
proto/lbm/collection/v1/genesis.proto Outdated Show resolved Hide resolved
tests/e2e/collection/grpc.go Show resolved Hide resolved
tests/e2e/collection/grpc.go Show resolved Hide resolved
tests/e2e/collection/grpc.go Show resolved Hide resolved
x/collection/msgs.go Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the A: build label Feb 27, 2024
@tkxkd0159 tkxkd0159 requested review from 0Tech, a team and tkxkd0159 February 27, 2024 10:19
@jaeseung-bae
Copy link
Contributor Author

LGTM (Since this PR was requested by me, I can't click approve by myself)

x/collection/client/cli/query.go Show resolved Hide resolved
x/collection/keeper/genesis_test.go Outdated Show resolved Hide resolved
x/collection/keeper/keeper_test.go Outdated Show resolved Hide resolved
x/collection/keeper/msg_server_test.go Show resolved Hide resolved
x/collection/module/module.go Show resolved Hide resolved
@tkxkd0159 tkxkd0159 requested a review from 0Tech March 4, 2024 01:27
0Tech
0Tech previously approved these changes Mar 4, 2024
tkxkd0159
tkxkd0159 previously approved these changes Mar 4, 2024
@tkxkd0159 tkxkd0159 dismissed stale reviews from 0Tech and themself via e93bf3e March 4, 2024 02:25
@tkxkd0159 tkxkd0159 merged commit 7aaa26e into bumpup50 Mar 5, 2024
17 of 18 checks passed
@tkxkd0159 tkxkd0159 deleted the bumpup50-collection branch March 5, 2024 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update x/collection to use Finschia/[email protected]
3 participants