Bump substrate v0.9.28 and add a MeetupResult in the IssuedRewards storage.#274
Bump substrate v0.9.28 and add a MeetupResult in the IssuedRewards storage.#274
MeetupResult in the IssuedRewards storage.#274Conversation
There was a problem hiding this comment.
Some minor comments. Otherwise, it looks good.
@pifragile you might also want to have a look at this.
Further, I am considering of changing the paradigm, such that we also have branch = polkadot-v9.x.x on master. Do you have any objections here @pifragile?
| info!(target: LOG, "marking issuance as completed for failed meetup."); | ||
|
|
||
| <IssuedRewards<T>>::insert((cid, cindex), meetup_index, meetup_result); | ||
| return Ok(Pays::No.into()) |
There was a problem hiding this comment.
We should also return the error here. Otherwise, the user expects some rewards.
There was a problem hiding this comment.
If we return an error, the storage (IssuedRewards) will not be updated. We did these changes to fix it: #275
The "error" feedback is in the MeetupResult:
MeetupResult::VotesNotDependable or MeetupResult::MeetupValidationIndexOutOfBounds. Perhaps we can improve the name: MeetupResult::VotesNotDependableError, MeetupResult::MeetupValidationIndexOutOfBoundsError ?
There was a problem hiding this comment.
Ah, sorry, I completely forgot that this was the cause of this PR. In that case, we should additionally add an event:
MeetupEvaluated(MeetupResult)
clangenb
left a comment
clangenb
left a comment
There was a problem hiding this comment.
One tiny remark, which will lead you to revert some stuff here. :)
ceremonies/src/lib.rs
Outdated
| Option<MeetupResult>, | ||
| ValueQuery, |
There was a problem hiding this comment.
Instead of making this an Option<MeetupResult>, you should change ValueQuery to OptionQuery.
clangenb
left a comment
clangenb
left a comment
There was a problem hiding this comment.
Thanks, looks good to me now!
| let meetup_result = IssuedRewards::<TestRuntime>::get((cid, cindex), 1); | ||
| assert_eq!(meetup_result, Some(MeetupResult::VotesNotDependable)); | ||
|
|
||
| assert!(event_deposited::<TestRuntime>( | ||
| Event::MeetupEvaluated(cid, 1, MeetupResult::VotesNotDependable).into() | ||
| )); |
clangenb
changed the title
MeetupResult in the IssuedRewards storage.
|
sorry i completely missed this @clangenb |
3 participants
Upgrade to polkadot-v0.9.28 :
Reasoning for adding the
MeetupResult:Failing calls do no longer change the storage, see #275. So we had to change the dispatch result to be OK, but in turn we store the
MeetupResultinIssuedRewardsinstead of just a plain(). This is anyhow helpful information for client applications. Further, an event is emitted when a failing meetup is evaluated.