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(launch): change param request #957

Merged
merged 24 commits into from
Sep 23, 2022
Merged

Conversation

aljo242
Copy link
Contributor

@aljo242 aljo242 commented Sep 19, 2022

Closes #930

@lubtd let me know what you think !

@aljo242 aljo242 changed the title Feat/change param request feat(launch): change param request Sep 19, 2022
@codecov
Copy link

codecov bot commented Sep 19, 2022

Codecov Report

Merging #957 (533361b) into develop (6f6ae7d) will decrease coverage by 0.10%.
The diff coverage is 4.92%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #957      +/-   ##
===========================================
- Coverage    10.66%   10.55%   -0.11%     
===========================================
  Files          272      275       +3     
  Lines        65229    66486    +1257     
===========================================
+ Hits          6954     7017      +63     
- Misses       58112    59300    +1188     
- Partials       163      169       +6     
Impacted Files Coverage Δ
x/launch/keeper/grpc_param_change.go 0.00% <0.00%> (ø)
x/launch/keeper/request.go 95.43% <0.00%> (-4.57%) ⬇️
x/launch/simulation/simulation.go 0.00% <0.00%> (ø)
x/launch/simulation/store.go 58.46% <0.00%> (-6.50%) ⬇️
x/launch/types/genesis.pb.go 1.02% <0.00%> (-0.08%) ⬇️
x/launch/types/params.go 76.31% <ø> (ø)
x/launch/types/query.pb.gw.go 0.00% <0.00%> (ø)
x/participation/simulation/simulation.go 0.00% <ø> (ø)
x/launch/types/request.pb.go 2.56% <0.30%> (-0.67%) ⬇️
x/launch/types/events.pb.go 0.48% <0.37%> (-0.02%) ⬇️
... and 6 more

proto/launch/events.proto Outdated Show resolved Hide resolved
proto/launch/request.proto Outdated Show resolved Hide resolved
x/launch/keeper/change_param.go Outdated Show resolved Hide resolved
x/launch/types/request_content.go Show resolved Hide resolved
Copy link
Contributor

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

We need to add logic in genesis.go for genesis export and import with ParamChange objects in the genesis state.

The easiest way to represent the genesis state would be to add LaunchID in ParamChange so information of type index is self-contained

message ParamChange {
  string module = 1;
  string param  = 2;
  bytes  value  = 3;
}

and adding in (m RequestContent) Validate(launchID uint64) verification of the launchID same as for GenesisAccount request

x/launch/keeper/change_param.go Outdated Show resolved Hide resolved
@aljo242 aljo242 requested a review from lumtis September 21, 2022 13:07
@aljo242
Copy link
Contributor Author

aljo242 commented Sep 21, 2022

Unit tests failing due to codecov upload

Copy link
Contributor

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

👍

x/launch/keeper/request.go Outdated Show resolved Hide resolved
x/participation/simulation/simulation.go Show resolved Hide resolved
Co-authored-by: Lucas Btd <lucas.bertrand.22@gmail.com>
Copy link
Contributor

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

We should add a query to list all param changes from a LaunchID

I think only the list query is sufficient, show query would not be necessary

@aljo242 aljo242 requested a review from lumtis September 22, 2022 19:35
Alex Johnson and others added 2 commits September 23, 2022 09:09
@aljo242 aljo242 requested a review from lumtis September 23, 2022 14:16
@lumtis lumtis merged commit 5086ccb into develop Sep 23, 2022
@lumtis lumtis deleted the feat/change-param-request branch September 23, 2022 14:28
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.

Implementing ChangeParam request
2 participants