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.

Sign up for GitHub

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

Jump to bottom

x/gamm: Internal audit and documentation #1858

Merged
merged 9 commits into from
Jun 30, 2022
Merged

x/gamm: Internal audit and documentation #1858

merged 9 commits into from
Jun 30, 2022

Conversation

AlpinYukseloglu
Copy link
Contributor

@AlpinYukseloglu AlpinYukseloglu commented Jun 23, 2022 •
edited by ValarDragon
Loading

Component of: #1835

What is the purpose of the change

Ongoing audit of x/gamm module for all AMM-related logic.

Current progress is being tracked in this document.

Brief Changelog

WIP, will add when complete.

Testing and Verifying

This change added tests and can be verified as follows:

  • WIP, will add when complete.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? no
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no
  • How is the feature or change documented? not applicable

@github-actions github-actions bot added the C:x/gamm Changes, features and bugs related to the gamm module. label Jun 23, 2022
@AlpinYukseloglu AlpinYukseloglu marked this pull request as ready for review June 30, 2022 05:54
@AlpinYukseloglu AlpinYukseloglu requested a review from a team June 30, 2022 05:54
This was referenced Jun 30, 2022
Closed
Closed
Closed
Closed
Closed
Open
mattverse
mattverse approved these changes Jun 30, 2022
View reviewed changes
Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

Overall LGTM! Left some minor comments, please take a look

x/gamm/keeper/genesis.go Outdated Show resolved Hide resolved
x/gamm/keeper/multihop.go Outdated Show resolved Hide resolved
alexanderbez
alexanderbez reviewed Jun 30, 2022
View reviewed changes
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

Nice work

x/gamm/keeper/genesis.go Outdated Show resolved Hide resolved
x/gamm/keeper/genesis.go Outdated Show resolved Hide resolved
x/gamm/keeper/multihop.go Outdated Show resolved Hide resolved
x/tokenfactory/keeper/admins_test.go Outdated Show resolved Hide resolved
x/tokenfactory/keeper/admins_test.go Outdated Show resolved Hide resolved
x/gamm/keeper/pool.go Outdated Show resolved Hide resolved
ValarDragon
ValarDragon approved these changes Jun 30, 2022
View reviewed changes
Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

This LGTM, lets have the subsequent work & followups we discussed on Monday in additional PRs?

@AlpinYukseloglu
Copy link
Contributor Author

AlpinYukseloglu commented Jun 30, 2022 •
edited
Loading

This LGTM, lets have the subsequent work & followups we discussed on Monday in additional PRs?

Sounds good!

@ValarDragon ValarDragon merged commit 15736e5 into main Jun 30, 2022
@ValarDragon ValarDragon deleted the gamm-audit branch June 30, 2022 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexanderbez alexanderbez alexanderbez left review comments

@mattverse mattverse mattverse approved these changes

@ValarDragon ValarDragon ValarDragon approved these changes

Assignees
No one assigned
Labels
C:x/gamm Changes, features and bugs related to the gamm module. C:x/tokenfactory
Projects
Osmosis Chain Development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AlpinYukseloglu @alexanderbez @ValarDragon @mattverse