Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Add tipping into treasury#4480

Merged
shawntabrizi merged 27 commits intomasterfrom
gav-treasury-tip
Jan 9, 2020
Merged

Add tipping into treasury#4480
shawntabrizi merged 27 commits intomasterfrom
gav-treasury-tip

Conversation

@gavofyork
Copy link
Member

@gavofyork gavofyork commented Dec 22, 2019 •
edited
Loading

Closes #4477

TODO:

  • Module-level docs
  • Function-level docs (with complexity)
  • Tests

@gavofyork gavofyork added the A3-in_progress Pull request is in progress. No review needed at this stage. label Dec 22, 2019
@kianenigma
Copy link
Contributor

kianenigma commented Dec 28, 2019

Custom weights(?)

#4158

@gavofyork gavofyork added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jan 4, 2020
gavofyork
gavofyork commented Jan 4, 2020
View reviewed changes
frame/support/src/traits.rs
}
fn contains(t: &T) -> bool { Self::sorted_members().binary_search(t).is_ok() }

impl<V: PartialEq, T: Get<V>> Contains<V> for T {
Copy link
Member Author

@gavofyork gavofyork Jan 4, 2020

Choose a reason for hiding this comment

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

this won't work if we also have specialisation for a module as we do now with the treasury module, hence the need for these changes and ord_parameter_types.

@gavofyork gavofyork requested a review from shawntabrizi January 8, 2020 21:33
shawntabrizi
shawntabrizi reviewed Jan 9, 2020
View reviewed changes
shawntabrizi
shawntabrizi reviewed Jan 9, 2020
View reviewed changes
shawntabrizi
shawntabrizi reviewed Jan 9, 2020
View reviewed changes
shawntabrizi
shawntabrizi reviewed Jan 9, 2020
View reviewed changes
shawntabrizi
shawntabrizi reviewed Jan 9, 2020
View reviewed changes
shawntabrizi
shawntabrizi reviewed Jan 9, 2020
View reviewed changes
shawntabrizi
shawntabrizi reviewed Jan 9, 2020
View reviewed changes
shawntabrizi
shawntabrizi reviewed Jan 9, 2020
View reviewed changes
shawntabrizi
shawntabrizi reviewed Jan 9, 2020
View reviewed changes
shawntabrizi
shawntabrizi reviewed Jan 9, 2020
View reviewed changes
shawntabrizi
shawntabrizi reviewed Jan 9, 2020
View reviewed changes
shawntabrizi
shawntabrizi reviewed Jan 9, 2020
View reviewed changes
shawntabrizi
shawntabrizi approved these changes Jan 9, 2020
View reviewed changes
Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

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

Some questions and nit feedback, but this looks good to me.

Very wholesome feature :)

shawntabrizi
shawntabrizi suggested changes Jan 9, 2020
View reviewed changes
Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

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

Oops, actually you forgot to include base weight values for the new extrinsics.

@shawntabrizi shawntabrizi added A5-grumble and removed A0-please_review Pull request needs code review. labels Jan 9, 2020
gavofyork and others added 5 commits January 9, 2020 13:44
@gavofyork @shawntabrizi
Update frame/treasury/src/lib.rs
7d8e249 
Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com>
@gavofyork @shawntabrizi
Update frame/treasury/src/lib.rs
834a7ba 
Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com>
@gavofyork @shawntabrizi
Update frame/treasury/src/lib.rs
73357e2 
Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com>
@gavofyork @shawntabrizi
Apply suggestions from code review
b95e6d5 
Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com>
@gavofyork
Add provisional weights.
c0f8d64
@gavofyork gavofyork added A0-please_review Pull request needs code review. and removed A5-grumble labels Jan 9, 2020
@gavofyork gavofyork requested a review from shawntabrizi January 9, 2020 12:56
@shawntabrizi shawntabrizi added A8-looksgood and removed A0-please_review Pull request needs code review. labels Jan 9, 2020
@shawntabrizi shawntabrizi merged commit e81f7c6 into master Jan 9, 2020
@shawntabrizi shawntabrizi deleted the gav-treasury-tip branch January 9, 2020 13:39
wliyongfeng
wliyongfeng reviewed Dec 29, 2020
View reviewed changes
frame/treasury/src/lib.rs
if Self::insert_tip_and_check_closing(&mut tip, tipper, tip_value) {
Self::deposit_event(RawEvent::TipClosing(hash.clone()));
}
Tips::<T>::insert(&hash, tip);
Copy link

@wliyongfeng wliyongfeng Dec 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry for comment with this historical PR, but I want to ask were there any special considerations without emitting a Tip event here? Just to avoid too many events?

Actually it's a little troublesome without this event to handle off-chain business.

Copy link
Contributor

@gui1117 gui1117 Dec 29, 2020

Choose a reason for hiding this comment

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

I think we can emit some events if needed, it shouldn't be too may events AFAICT. You can open an issue or PR

Copy link

@wliyongfeng wliyongfeng Dec 29, 2020

Choose a reason for hiding this comment

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

I think we can emit some events if needed, it shouldn't be too may events AFAICT. You can open an issue or PR

I see. Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@kianenigma kianenigma Awaiting requested review from kianenigma

@gui1117 gui1117 Awaiting requested review from gui1117

2 more reviewers

@wliyongfeng wliyongfeng wliyongfeng left review comments

@shawntabrizi shawntabrizi shawntabrizi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tip request in treasury

5 participants

@gavofyork @kianenigma @shawntabrizi @wliyongfeng @gui1117