feat!: implement new treasury account #1067

dadamu · 2023-01-11T12:00:21Z

Description

Closes: #XXXX
This PR implements ADR-018 to apply the new treasury account structure and its methods.

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...

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 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)

…treasury-impl

codecov · 2023-01-12T13:36:41Z

Codecov Report

Base: 80.23% // Head: 80.38% // Increases project coverage by +0.14% 🎉

Coverage data is based on head (0cd9823) compared to base (8c9058b).
Patch coverage: 93.75% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1067      +/-   ##
==========================================
+ Coverage   80.23%   80.38%   +0.14%     
==========================================
  Files         176      179       +3     
  Lines       15427    15604     +177     
==========================================
+ Hits        12378    12543     +165     
- Misses       2508     2516       +8     
- Partials      541      545       +4

Impacted Files	Coverage Δ
x/subspaces/types/msgs.go	`98.29% <ø> (-0.03%)`	⬇️
x/subspaces/types/permissions.go	`77.77% <ø> (ø)`
x/subspaces/types/codec.go	`58.82% <60.00%> (+0.12%)`	⬆️
x/subspaces/keeper/alias_functions.go	`81.25% <75.00%> (-0.24%)`	⬇️
x/subspaces/legacy/v5/store.go	`89.28% <89.28%> (ø)`
x/subspaces/keeper/msg_server_treasury.go	`95.89% <95.89%> (ø)`
x/subspaces/types/msgs_treasury.go	`96.77% <96.77%> (ø)`
app/app.go	`88.04% <100.00%> (ø)`
x/subspaces/keeper/keeper.go	`100.00% <100.00%> (ø)`
x/subspaces/keeper/msg_server.go	`88.41% <100.00%> (+0.05%)`	⬆️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Makefile

RiccardoM · 2023-01-13T18:48:03Z

x/subspaces/client/cli/tx_treasury.go

+}
+
+// getStakeAuthorization returns a send authorization from the given command flags
+func getSendAuthorization(flags *pflag.FlagSet) (*banktypes.SendAuthorization, error) {


I guess this method and the other ones are all copy/pasted from the Cosmos SDK. Do you think we should make them public inside our fork instead? This way we don't need to re-write them here but we can simply rely on those. We could also upstreami this change to the Cosmos SDK repo so that they are by default public for everyone to use

Yes, it is better that we make it public in our Cosmos-SDK fork. I created a pull request in that repo:
desmos-labs/cosmos-sdk#148

After the authz change merged into our fork and is released, we can apply the feature in this PR.

x/subspaces/keeper/alias_functions.go

x/subspaces/keeper/msg_server_treasury.go

x/subspaces/keeper/msg_server_treasury_test.go

x/subspaces/legacy/v3/store.go

RiccardoM · 2023-01-13T18:58:17Z

x/subspaces/legacy/v5/store_test.go

+				require.Equal(t, types.NewSubspace(
+					1,
+					"name",
+					"description",
+					"cosmos1cyjzgj9j7d2gdqk78pa0fgvfnlzradat97aek9",
+					"cosmos10ya9y35qkf4puaklx5fs07sxfxqncx9usgsnz6",
+					"cosmos10ya9y35qkf4puaklx5fs07sxfxqncx9usgsnz6",
+					time.Date(2023, 1, 11, 1, 1, 1, 1, time.UTC),
+				), newSubspace)


Can we use types.SubspaceTreasuryAddress here instead of a text value? This way we can make sure the address is generated properly inside the migration code by calling the same function

We should remain text generated by current method, it would detect the change and make sure the migration always working properly if types.GetTreasuryAddress or authtypes.NewModuleAddress are changed.

RiccardoM · 2023-01-13T19:03:39Z

x/subspaces/types/models_test.go

@@ -583,3 +581,7 @@ func TestUserGroup_Update(t *testing.T) {
 		})
 	}
 }
+
+func TestGetTreasuryAddress(t *testing.T) {
+	require.Equal(t, "cosmos1cyjzgj9j7d2gdqk78pa0fgvfnlzradat97aek9", types.GetTreasuryAddress(1).String())


I believe we should test this method more properly. Ideally, what we could do is:

Generate a list of subspace ids

For each subspace id, generate the treasury address

Check that the generated addresses are all different from one another

This will make sure that we are not only always testing one case (SubspaceID: 1) but we are actually proving the generation method works for every subspace id

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

…treasury-impl

RiccardoM

Looks good to me! Great job 💯

RiccardoM · 2023-01-17T13:53:11Z

@dadamu Can you just add a changelog entry for this change please?

dadamu added 6 commits January 5, 2023 20:26

feat: use new treasury address

408dcea

feat: build poc of treasury authorization

37c2413

Merge branch 'master' of github.com:desmos-labs/desmos into paul/new-…

d13854b

…treasury-impl

feat: build minimal features

512ca6a

tests: add types tests

c8615dc

feat: implement msg server and tests

ae9971e

github-actions bot added kind/build Related to the build of the project x/CLI x/subspaces Issue on the x/subspaces module labels Jan 11, 2023

dadamu added 6 commits January 12, 2023 15:33

test: add cli tests

c6c4f0b

feat: add wasm types and tests

380ced2

fix: readd cli go:build config

d2d89f3

tset: fix other modules tests

a255b86

feat: add migration and its tests

eee7777

feat: register new migration to module

81bb189

github-actions bot added x/profiles Module that allows to create and manage decentralized social profiles x/relationships labels Jan 12, 2023

dadamu added 9 commits January 12, 2023 20:53

test: add simulation tests

1fb7e0c

fix: fix sim test grant treasury authorization msg

b3d1a61

fix: fix permission naming

c2b8a39

fix: fix error msgs

fc1c534

fix: add missing docs and params

6f62015

docs: add missing docs

75e093d

docs: fix words

d636dae

Merge branch 'master' of github.com:desmos-labs/desmos into paul/new-…

f3238fe

…treasury-impl

test: fix profiles test error

14a410f

fix: add missing error check

e454fd2

dadamu marked this pull request as ready for review January 12, 2023 14:43

dadamu requested a review from a team as a code owner January 12, 2023 14:43

dadamu requested review from manu0466 and RiccardoM January 12, 2023 15:08

RiccardoM reviewed Jan 13, 2023

View reviewed changes

dadamu and others added 15 commits January 17, 2023 14:19

Update x/subspaces/keeper/msg_server_treasury.go

1a93161

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

Update x/subspaces/keeper/alias_functions.go

399e1a1

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

Update x/subspaces/keeper/msg_server_treasury.go

3af4e97

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

Update x/subspaces/types/permissions.go

f0cbfdc

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

Update x/subspaces/keeper/msg_server_treasury_test.go

b7110bf

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

Update x/subspaces/keeper/msg_server_treasury_test.go

5f4993e

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

Update x/subspaces/keeper/msg_server_treasury_test.go

c5cdf09

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

Update x/subspaces/keeper/msg_server_treasury_test.go

ec6297d

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

Update x/subspaces/keeper/msg_server_treasury_test.go

7e167a3

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

Update x/subspaces/keeper/msg_server_treasury_test.go

09e4cfc

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

Update x/subspaces/keeper/msg_server_treasury_test.go

ab14d15

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

Update x/subspaces/keeper/msg_server_treasury_test.go

3951313

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

chore: improve creatAccount function name

095e527

Merge branch 'master' of github.com:desmos-labs/desmos into paul/new-…

c029857

…treasury-impl

test: apply suggestions

0cd9823

RiccardoM approved these changes Jan 17, 2023

View reviewed changes

RiccardoM added the automerge Automatically merge PR once all prerequisites pass label Jan 17, 2023

manu0466 approved these changes Jan 17, 2023

View reviewed changes

mergify bot merged commit 8981600 into master Jan 17, 2023

mergify bot deleted the paul/new-treasury-impl branch January 17, 2023 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: implement new treasury account #1067

feat!: implement new treasury account #1067

dadamu commented Jan 11, 2023 •

edited

codecov bot commented Jan 12, 2023 •

edited

RiccardoM Jan 13, 2023

dadamu Jan 17, 2023

RiccardoM Jan 13, 2023

dadamu Jan 17, 2023 •

edited

RiccardoM Jan 13, 2023

dadamu Jan 17, 2023

RiccardoM left a comment

RiccardoM commented Jan 17, 2023

feat!: implement new treasury account #1067

feat!: implement new treasury account #1067

Conversation

dadamu commented Jan 11, 2023 • edited

Description

Author Checklist

Reviewers Checklist

codecov bot commented Jan 12, 2023 • edited

Codecov Report

RiccardoM Jan 13, 2023

Choose a reason for hiding this comment

dadamu Jan 17, 2023

Choose a reason for hiding this comment

RiccardoM Jan 13, 2023

Choose a reason for hiding this comment

dadamu Jan 17, 2023 • edited

Choose a reason for hiding this comment

RiccardoM Jan 13, 2023

Choose a reason for hiding this comment

dadamu Jan 17, 2023

Choose a reason for hiding this comment

RiccardoM left a comment

Choose a reason for hiding this comment

RiccardoM commented Jan 17, 2023

dadamu commented Jan 11, 2023 •

edited

codecov bot commented Jan 12, 2023 •

edited

dadamu Jan 17, 2023 •

edited