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

Initial treasury docs #5678

Merged
merged 16 commits into from
Mar 22, 2024
Merged

Initial treasury docs #5678

merged 16 commits into from
Mar 22, 2024

Conversation

CrackTheCode016
Copy link
Contributor

@CrackTheCode016 CrackTheCode016 commented Mar 13, 2024

  • Adds the concept of sub-treasuries
  • Update treasury guide to use the spend_local call rather than the spend call
  • Explains the notion of multi-asset treasuries (follow up PR to come...)

@CrackTheCode016 CrackTheCode016 added A1 - In Progress Not ready for review yet. A0 - Do Not Merge Pull request should not yet be merged. labels Mar 13, 2024
@filippoweb3 filippoweb3 linked an issue Mar 14, 2024 that may be closed by this pull request
@filippoweb3
Copy link
Contributor

@CrackTheCode016 I linked the issue about spend local calls. I think we can address that issue with this PR. Wdyt?

@CrackTheCode016
Copy link
Contributor Author

@filippoweb3 yup we can! I think I'll split this into two PRs:

  1. Take care of updating spend local calls, sub treasuries, and spending different kinds of assets via spend
  2. Second PR for actual guides to update and do the above. WDYT?

@CrackTheCode016 CrackTheCode016 added A2 - Please Review Pull request is ready for review. and removed A1 - In Progress Not ready for review yet. A0 - Do Not Merge Pull request should not yet be merged. labels Mar 14, 2024
Copy link
Contributor

@filippoweb3 filippoweb3 left a comment

Choose a reason for hiding this comment

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

I added conditional rendering for Kusama, some minor edits and some suggestions to make the additions a bit more clear to the reader. Let me know what you think!

docs/learn/learn-polkadot-opengov-treasury.md Outdated Show resolved Hide resolved
docs/learn/learn-polkadot-opengov-treasury.md Outdated Show resolved Hide resolved
docs/learn/learn-polkadot-opengov-treasury.md Outdated Show resolved Hide resolved
docs/learn/learn-polkadot-opengov-treasury.md Outdated Show resolved Hide resolved
docs/learn/learn-polkadot-opengov-treasury.md Outdated Show resolved Hide resolved
docs/learn/learn-polkadot-opengov-treasury.md Outdated Show resolved Hide resolved
docs/learn/learn-polkadot-opengov-treasury.md Outdated Show resolved Hide resolved
docs/learn/learn-polkadot-opengov-treasury.md Outdated Show resolved Hide resolved
docs/learn/learn-polkadot-opengov-treasury.md Show resolved Hide resolved
docs/learn/learn-polkadot-opengov-treasury.md Show resolved Hide resolved
@CrackTheCode016
Copy link
Contributor Author

@filippoweb3 Besides the liquidity aspect, the PR is ready, but we can wait on that specifically before reviewing again.

@CrackTheCode016 CrackTheCode016 requested a review from DrW3RK March 19, 2024 19:28
Copy link
Contributor

@filippoweb3 filippoweb3 left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for implementing the feedback!

docs/learn/learn-polkadot-opengov-treasury.md Outdated Show resolved Hide resolved
docs/learn/learn-polkadot-opengov-treasury.md Outdated Show resolved Hide resolved
docs/learn/learn-polkadot-opengov-treasury.md Outdated Show resolved Hide resolved
Comment on lines 107 to 108
3. The asset has a set conversion rate, as per OpenGov referenda on the Treasurer track (set via the
asset rate pallet). This conversion rate defines a fixed-rate representation for converting from
Copy link
Member

Choose a reason for hiding this comment

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

Is this true? How can the conversion rate be fixed, given the assets are volatile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I meant a fixed point, not a fixed rate, although I think that is the case anyway, as the rate is a FixedI128 set in the StorageMap and can be updated. not sure what the strategy is to maintain it unless I am misunderstanding something.

For more detail, take a look here, and how the rate is set in the pallet: https://github.com/paritytech/polkadot-sdk/blob/475e7a147676a4e7a9d255ddc7969dd35ea22882/substrate/frame/asset-rate/src/lib.rs#L18

@DrW3RK DrW3RK merged commit 9f7b6e3 into master Mar 22, 2024
3 checks passed
@DrW3RK DrW3RK deleted the more-treasury-docs branch March 22, 2024 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A2 - Please Review Pull request is ready for review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spend and Spend Local calls - Treasury
3 participants