Rework governance/ update admin-module NTRN-70 #303

quasisamurai · 2023-08-15T09:33:47Z

Admin module PR
Neutron PR
Neutron SDK PR
Integration tests

pr0n00gler · 2023-08-17T12:34:35Z

app/app.go

+	// TODO: enabled proposals?
+	//if len(enabledProposals) != 0 {
+	//	app.AdminmoduleKeeper.Router().AddRoute(wasm.RouterKey, wasm.NewWasmProposalHandler(app.WasmKeeper, enabledProposals))
+	//}


app/app.go

wasmbinding/message_plugin.go

# Conflicts: # go.mod # go.sum

pr0n00gler · 2023-08-25T10:16:05Z

app/app.go

 		wasmOpts...,
 	)
 	wasmHooks.ContractKeeper = &app.WasmKeeper

 	app.CronKeeper.WasmMsgServer = wasmkeeper.NewMsgServerImpl(&app.WasmKeeper)
 	cronModule := cron.NewAppModule(appCodec, &app.CronKeeper)

-	if len(enabledProposals) != 0 {


enabledProposals variable is not used anymore, remove it please

pr0n00gler · 2023-08-25T10:17:18Z

app/proposals_allowlisting.go

@@ -41,6 +36,20 @@ func isConsumerParamChangeWhitelisted(paramChanges []proposal.ParamChange) bool
 	return true
 }

+func isSdkMessageWhitelisted(msg sdk.Msg) bool {


please add a comment what is the purpose of this method.

pr0n00gler · 2023-08-25T10:29:31Z

app/proposals_allowlisting.go

@@ -41,6 +36,20 @@ func isConsumerParamChangeWhitelisted(paramChanges []proposal.ParamChange) bool
 	return true
 }

+func isSdkMessageWhitelisted(msg sdk.Msg) bool {
+	switch msg.(type) {


We definitely need to whitelist more messages here, but it's out of scope of this PR, will do it later

pr0n00gler · 2023-08-25T10:32:01Z

wasmbinding/message_plugin.go

@@ -295,6 +294,34 @@ func (m *CustomMessenger) submitTx(ctx sdk.Context, contractAddr sdk.AccAddress,
 }

 func (m *CustomMessenger) submitAdminProposal(ctx sdk.Context, contractAddr sdk.AccAddress, submitAdminProposal *bindings.SubmitAdminProposal) ([]sdk.Event, [][]byte, error) {
+	var data []byte
+	if submitAdminProposal.AdminProposal.ParamChangeProposal != nil || submitAdminProposal.AdminProposal.UpgradeProposal != nil || submitAdminProposal.AdminProposal.ClientUpdateProposal != nil {


Please add a comment there why exactly we need that condition.

Also i think it's cleaner to create a method isLegacyProposal() which does this check inside of it

These two conditions differ only in the method that should called (m.performSubmitAdminProposalLegacy() or m.performSubmitAdminProposal()).

Logging and unmarshalling logic is the same for both conditions. Let's refactor to make it easier to read?

wasmbinding/message_plugin.go

Co-authored-by: Mike Mozhaev <programmer10110@gmail.com>

wasmbinding/message_plugin.go

swelf19 · 2023-08-29T13:04:09Z

app/app.go

@@ -170,23 +169,6 @@ var (
 	EnableSpecificProposals = ""
 )

-// GetEnabledProposals parses the ProposalsEnabled / EnableSpecificProposals values to
-// produce a list of enabled proposals to pass into wasmd app.
-func GetEnabledProposals() []wasm.ProposalType {


do you think its safe to remove EnableSpecificProposals and ProposalsEnabled variables. and also remove all mentions in Makefile

why it might be unsafe? wasm.ProposalType is basically deprecated

wasmbinding/message_plugin.go

app/app.go

wasmbinding/message_plugin.go

NeverHappened

There are lots of app.GetEnabledProposals(), in testing code that needs to be fixed

app/app.go

quasisamurai added 4 commits August 15, 2023 13:07

refactor known proposals

d0b83b7

ref & add new binding

0512cef

update admin module

73025c5

smol upd

b86ff05

quasisamurai force-pushed the feat/admin-module-sdk47 branch from f400a7f to b86ff05 Compare August 15, 2023 10:55

quasisamurai added 3 commits August 15, 2023 14:57

gosum

bf6aed9

upd module

f5fb999

plugin errors fix

1aa5d97

quasisamurai changed the title ~~Feat/admin module sdk47~~ admin module & governance sdk47 update NTRN-70 Aug 17, 2023

quasisamurai marked this pull request as ready for review August 17, 2023 08:28

This was referenced Aug 17, 2023

Rework governance/ update admin-module NTRN-70 (sdk-47) neutron-org/neutron-sdk#107

Merged

Feat/admin module sdk47 NTRN-70 neutron-org/admin-module#2

Merged

pr0n00gler requested changes Aug 17, 2023

View reviewed changes

quasisamurai added 3 commits August 18, 2023 12:05

Merge branch 'update-sdk47' into feat/admin-module-sdk47

df993bc

# Conflicts: # go.mod # go.sum

rework to get rid of rudimentary proposals

143c6c9

add whitelisting, fix issues, upd adminmodule

fc3e6fc

quasisamurai mentioned this pull request Aug 25, 2023

Rework governance/ update admin-module NTRN-70 sdk47 neutron-org/neutron-integration-tests#196

Merged

rm legacy code

7c2ea7a

quasisamurai changed the title ~~admin module & governance sdk47 update NTRN-70~~ Rework governance/ update admin-module NTRN-70 Aug 25, 2023

pr0n00gler requested changes Aug 25, 2023

View reviewed changes

quasisamurai and others added 4 commits August 29, 2023 15:17

Update wasmbinding/message_plugin.go

7397bd9

Co-authored-by: Mike Mozhaev <programmer10110@gmail.com>

Update wasmbinding/message_plugin.go

696ad10

Co-authored-by: Mike Mozhaev <programmer10110@gmail.com>

Update wasmbinding/message_plugin.go

ccf0c53

Co-authored-by: Mike Mozhaev <programmer10110@gmail.com>

post-review & small refactor

552a67f

quasisamurai force-pushed the feat/admin-module-sdk47 branch from 845b949 to 552a67f Compare August 29, 2023 12:02

add comment

427455a

sotnikov-s reviewed Aug 29, 2023

View reviewed changes

wasmbinding/message_plugin.go Outdated Show resolved Hide resolved

wasmbinding/message_plugin.go Outdated Show resolved Hide resolved

wasmbinding/message_plugin.go Outdated Show resolved Hide resolved

wasmbinding/message_plugin.go Show resolved Hide resolved

swelf19 reviewed Aug 30, 2023

View reviewed changes

quasisamurai added 2 commits August 30, 2023 13:12

post-review

cf7b0ec

polishing

bbc0480

rm enabled proposals

b5647e2

sotnikov-s reviewed Aug 30, 2023

View reviewed changes

wasmbinding/message_plugin.go Show resolved Hide resolved

wasmbinding/message_plugin.go Outdated Show resolved Hide resolved

wasmbinding/message_plugin.go Outdated Show resolved Hide resolved

sotnikov-s reviewed Aug 30, 2023

View reviewed changes

wasmbinding/message_plugin.go Outdated Show resolved Hide resolved

small ref & comment

2898efb

NeverHappened reviewed Aug 30, 2023

View reviewed changes

app/app.go Outdated Show resolved Hide resolved

NeverHappened reviewed Aug 31, 2023

View reviewed changes

app/app.go Outdated Show resolved Hide resolved

NeverHappened reviewed Aug 31, 2023

View reviewed changes

wasmbinding/message_plugin.go Outdated Show resolved Hide resolved

quasisamurai added 2 commits September 4, 2023 11:31

upd module & small fixes

95adbda

upda admin module

0489bc1

NeverHappened reviewed Sep 5, 2023

View reviewed changes

app/app.go Outdated Show resolved Hide resolved

upd module

1016569

pr0n00gler approved these changes Sep 6, 2023

View reviewed changes

sotnikov-s approved these changes Sep 6, 2023

View reviewed changes

quasisamurai added 4 commits September 6, 2023 18:58

Merge branch 'update-sdk47' into feat/admin-module-sdk47

71dd6d4

upd module

8ad6829

rm getenabledproposals from tests

f549fc1

rm comma

dda51cd

pr0n00gler merged commit 743fc8f into update-sdk47 Sep 7, 2023
2 of 5 checks passed

pr0n00gler deleted the feat/admin-module-sdk47 branch May 9, 2024 18:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework governance/ update admin-module NTRN-70 #303

Rework governance/ update admin-module NTRN-70 #303

quasisamurai commented Aug 15, 2023 •

edited

Loading

pr0n00gler Aug 17, 2023

pr0n00gler Aug 25, 2023

pr0n00gler Aug 25, 2023

pr0n00gler Aug 25, 2023

pr0n00gler Aug 25, 2023

pr0n00gler Aug 25, 2023

pr0n00gler Aug 25, 2023

swelf19 Aug 29, 2023

quasisamurai Aug 30, 2023

NeverHappened left a comment

Rework governance/ update admin-module NTRN-70 #303

Rework governance/ update admin-module NTRN-70 #303

Conversation

quasisamurai commented Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NeverHappened left a comment

Choose a reason for hiding this comment

quasisamurai commented Aug 15, 2023 •

edited

Loading