Add necessary reward event #89

boundless-forest · 2022-05-20T10:42:41Z

Add two events to support Apps presentation easily:

/// Create new order. \[lane_id, message_nonce, assigned_relayers, order_fee, out_of_slots_time\]
OrderCreated(
	LaneId,
	MessageNonce,
	BalanceOf<T, I>,
	Vec<T::AccountId>,
	Option<T::BlockNumber>,
),
/// Reward distribute of the order. \[lane_id, message_nonce, rewards\]
OrderReward(LaneId, MessageNonce, RewardItem<T::AccountId, BalanceOf<T, I>>),

boundless-forest · 2022-05-24T06:55:38Z

@hackfisher @AurevoirXavier PTAR

modules/fee-market/src/lib.rs

hackfisher · 2022-05-30T09:22:26Z

modules/fee-market/src/s2s/payment.rs

+				Pallet::<T, I>::deposit_event(Event::OrderReward(
+					lane_id,
+					message_nonce,
+					reward_item.clone(),
+				));


I'm not sure whether we should deposit event in this function(slash_and_calculate_rewards) or at the end of pay_relayers_rewards?

If deposit this event at the end of pay_relayers_rewards, you may need to remove the RewardBook and iterate the whole RewardItems. I prefer the current way.

hackfisher · 2022-05-30T09:23:37Z

modules/fee-market/src/s2s/payment.rs

+	fn add_reward_item(&mut self, item: RewardItem<T::AccountId, BalanceOf<T, I>>) {
+		if let Some((id, reward)) = item.to_slot_relayer {
+			self.assigned_relayers_sum
+				.entry(id)
+				.and_modify(|r| *r = r.saturating_add(reward))
+				.or_insert(reward);
+		}
+
+		if let Some(reward) = item.to_treasury {
+			self.treasury_sum = self.treasury_sum.saturating_add(reward);
+		}
+
+		if let Some((id, reward)) = item.to_message_relayer {
+			self.deliver_sum
+				.entry(id)
+				.and_modify(|r| *r = r.saturating_add(reward))
+				.or_insert(reward);
+		}
+
+		if let Some((_id, reward)) = item.to_confirm_relayer {
+			self.confirm_sum = self.confirm_sum.saturating_add(reward);
+		}
+	}


Another way would be directly iterate though RewardItem and calculate the value wanted. RewardBook can be removed then.

We can do this. Store the reward item in something like HashMap and iterate it in the pay_relayers_rewards. But I think the reward book is easier to read and understand.

My feelings from my point as a reviewer, RewardBook is unnecessary concept and removing it will be simpler.

I'm going to merge this and review this discussion in separate issue because its blocking other release tasks.

hackfisher · 2022-05-30T09:24:22Z

@JayJay1024 Can these events meet your requirement?

JayJay1024 · 2022-06-09T03:01:42Z

@JayJay1024 Can these events meet your requirement?

yeah these two events seems to meet the requirement

boundless-forest · 2022-06-09T05:44:14Z

@hackfisher Please take a review again. I plan to do companions this afternoon and introduce this change to the darwinia-commoon side before the next release.

* Refactor the total reward cal * Code Clean * Fix test * Add OrderCreated event * Self review * Update comment * Update OrderCreate event to include relayers * Fix review

* Companion for 96, fix benchmark * Check order capacity when relayer enroll (#93) * Add check logic * Update test file * Add necessary reward event (#89) * Refactor the total reward cal * Code Clean * Fix test * Add OrderCreated event * Self review * Update comment * Update OrderCreate event to include relayers * Fix review

* Refactor the total reward cal * Code Clean * Fix test * Add OrderCreated event * Self review * Update comment * Update OrderCreate event to include relayers * Fix review

* Refactor the total reward cal * Code Clean * Fix test * Add OrderCreated event * Self review * Update comment * Update OrderCreate event to include relayers * Fix review # Conflicts: # modules/fee-market/src/s2s/payment.rs # modules/fee-market/src/tests.rs

* Refactor the total reward cal * Code Clean * Fix test * Add OrderCreated event * Self review * Update comment * Update OrderCreate event to include relayers * Fix review

* Update market after order is created or comfirmed (#94) * Update order capacity * FIX CI * Update comment * Redesign fee market payment (#169) * Add basic solution * Refactor Slash Report * Refactor Slash Report 2 * Add other changes * Add tests * Self review * Rename to `message_and_confirm_reward` * Rename to `previous_relayers` * Rename to AssignedRelayer * Add more comments * Add more comments * Add new reward implementation * Rename and clean the code, needs more test * Add more docs here * Prepare for tests * Fix broken tests * Refactor * Rename * Remove RewardBook * Self review * Save one storage * Try fix ci * Handle Zero `CollateralPerOrder` (#176) * Companion 189 for darwinia-parachain (#194) * Update fee-market terminology (#192) (#199) * Support pangolin <> pangolin parachain alpha bridge (#190) * Cherry-pick #201 to `darwinia-parachain` (#203) * Del useless related to fee calculation (#201) * Delete estimate_delivery_transaction * Delete transaction_payment * Delete MessageTransaction * Code clean * Fix compile * Anchor polkadot-v0.9.27 * remove unused import * Fix test problem (#172) * fix ci * Add necessary reward event (#89) * Refactor the total reward cal * Code Clean * Fix test * Add OrderCreated event * Self review * Update comment * Update OrderCreate event to include relayers * Fix review * Enhance dispatch module (#121) * rm shift session manager pallet and clean useless imports (#128) * `Pre-dispatch` validate for main branch (#130) * Adjust the traits * Try fix compile * Fix tests * Code clean * Avoid duplicate evm transact fees (#136) * cherry pick #137 (#140) * cherry pick #137 * pick #141 Co-authored-by: bear <[email protected]> * Drop error in `pre_dispatch` (#152) (#153) * Migrate to new s2s bridge (#149) * Add client-darwinia/client-crab/client-crab-parachain * Update CI * rever substrate commit * fix compile * remove transactional * Fix * Bump finality-grandpa to v0.16.0 * fix wrong import * make change follow paritytech/substrate#11415 * Add runtime-common to workspace and try fix test * Make changes follow paritytech/substrate#10242 * Code clean * More ci check * Remove integrity Co-authored-by: bear <[email protected]> * Companion for #155 (#157) * Update market after order is created or comfirmed (#94) * Update order capacity * FIX CI * Update comment * Redesign fee market payment (#169) * Add basic solution * Refactor Slash Report * Refactor Slash Report 2 * Add other changes * Add tests * Self review * Rename to `message_and_confirm_reward` * Rename to `previous_relayers` * Rename to AssignedRelayer * Add more comments * Add more comments * Add new reward implementation * Rename and clean the code, needs more test * Add more docs here * Prepare for tests * Fix broken tests * Refactor * Rename * Remove RewardBook * Self review * Save one storage * Try fix ci * Handle Zero `CollateralPerOrder` (#176) * Update Fee market docs (#178) * Update doc * Update example * Move to `dev-dependencies` (#182) * Sync part.1 (#184) * Remove fee relates (#186) * Sync missing changes from different branches (#191) * Support pangolin <> pangolin parachain alpha bridge (#190) * Integrate `fee-market` to `FromThisChainMessageVerifier` (#189) * Run ignored crate * Fix tests * Add features * Fix compile * Update fee-market terminology (#192) * Del useless related to fee calculation (#201) * Delete estimate_delivery_transaction * Delete transaction_payment * Delete MessageTransaction * Code clean * master > polkadot-v0.9.27 * roll back bp-parachain Co-authored-by: bear <[email protected]> Co-authored-by: Xavier Lau <[email protected]> Co-authored-by: Aki Wu <[email protected]> Co-authored-by: HackFisher <[email protected]> Co-authored-by: HackFisher <[email protected]> Co-authored-by: fewensa <[email protected]>

boundless-forest added 2 commits May 20, 2022 18:23

Refactor the total reward cal

7a28ba7

Code Clean

92b28b7

boundless-forest linked an issue May 20, 2022 that may be closed by this pull request

Add more necessary events to the fee market #87

Closed

boundless-forest added 4 commits May 23, 2022 14:13

Fix test

7a3330b

Add OrderCreated event

7ddeec6

Self review

644e126

Update comment

118fa9c

boundless-forest marked this pull request as ready for review May 23, 2022 08:09

boundless-forest requested review from hackfisher, JayJay1024 and wuminzhe May 23, 2022 08:09

boundless-forest marked this pull request as draft May 23, 2022 09:19

Update OrderCreate event to include relayers

4f2f202

boundless-forest marked this pull request as ready for review May 23, 2022 09:43

wuminzhe reviewed May 30, 2022

View reviewed changes

modules/fee-market/src/lib.rs Outdated Show resolved Hide resolved

boundless-forest added 2 commits May 30, 2022 13:54

Merge branch 'main' into bear-add-more-events

89f3c29

Fix review

622aeab

hackfisher reviewed May 30, 2022

View reviewed changes

hackfisher requested a review from AurevoirXavier May 30, 2022 09:24

JayJay1024 approved these changes Jun 9, 2022

View reviewed changes

hackfisher mentioned this pull request Jun 9, 2022

Another way would be directly iterate though RewardItem and calculate the value wanted. RewardBook can be removed then. #112

Closed

hackfisher merged commit 6ac478b into main Jun 9, 2022

hackfisher deleted the bear-add-more-events branch June 9, 2022 09:50

boundless-forest added a commit that referenced this pull request Jun 9, 2022

Add necessary reward event (#89)

1cf33be

* Refactor the total reward cal * Code Clean * Fix test * Add OrderCreated event * Self review * Update comment * Update OrderCreate event to include relayers * Fix review

jiguantong pushed a commit that referenced this pull request Jun 22, 2022

Add necessary reward event (#89)

34fd700

* Refactor the total reward cal * Code Clean * Fix test * Add OrderCreated event * Self review * Update comment * Update OrderCreate event to include relayers * Fix review

jiguantong pushed a commit that referenced this pull request Jul 1, 2022

Add necessary reward event (#89)

5bf70ac

* Refactor the total reward cal * Code Clean * Fix test * Add OrderCreated event * Self review * Update comment * Update OrderCreate event to include relayers * Fix review

jiguantong pushed a commit that referenced this pull request Sep 29, 2022

Add necessary reward event (#89)

78a6bbc

* Refactor the total reward cal * Code Clean * Fix test * Add OrderCreated event * Self review * Update comment * Update OrderCreate event to include relayers * Fix review

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add necessary reward event #89

Add necessary reward event #89

boundless-forest commented May 20, 2022 •

edited

Loading

boundless-forest commented May 24, 2022

hackfisher May 30, 2022 •

edited

Loading

boundless-forest Jun 9, 2022

hackfisher May 30, 2022 •

edited

Loading

boundless-forest Jun 9, 2022

hackfisher Jun 9, 2022 •

edited

Loading

hackfisher commented May 30, 2022

JayJay1024 commented Jun 9, 2022

boundless-forest commented Jun 9, 2022

Add necessary reward event #89

Add necessary reward event #89

Conversation

boundless-forest commented May 20, 2022 • edited Loading

boundless-forest commented May 24, 2022

hackfisher May 30, 2022 • edited Loading

Choose a reason for hiding this comment

boundless-forest Jun 9, 2022

Choose a reason for hiding this comment

hackfisher May 30, 2022 • edited Loading

Choose a reason for hiding this comment

boundless-forest Jun 9, 2022

Choose a reason for hiding this comment

hackfisher Jun 9, 2022 • edited Loading

Choose a reason for hiding this comment

hackfisher commented May 30, 2022

JayJay1024 commented Jun 9, 2022

boundless-forest commented Jun 9, 2022

boundless-forest commented May 20, 2022 •

edited

Loading

hackfisher May 30, 2022 •

edited

Loading

hackfisher May 30, 2022 •

edited

Loading

hackfisher Jun 9, 2022 •

edited

Loading