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

Companion for Decouple Staking and Election - Part 3: Signed Phase #2793

Merged
merged 20 commits into from
Jun 28, 2021
Merged

Companion for Decouple Staking and Election - Part 3: Signed Phase #2793

merged 20 commits into from
Jun 28, 2021

Conversation

coriolinus
Copy link
Contributor

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Apr 1, 2021
@coriolinus coriolinus mentioned this pull request Apr 1, 2021
Merged
@kianenigma
Copy link
Contributor

needs update.

@kianenigma kianenigma mentioned this pull request Jun 3, 2021
Merged
kianenigma
kianenigma suggested changes Jun 22, 2021
View reviewed changes
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Needs another more diligent update.

kianenigma
kianenigma reviewed Jun 22, 2021
View reviewed changes
runtime/kusama/src/lib.rs
type SignedDepositByte = SignedDepositByte;
type SignedDepositWeight = ();
type SignedMaxWeight = Self::MinerMaxWeight;
type SlashHandler = (); // burn slashes
Copy link
Contributor

Choose a reason for hiding this comment

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

Same, could use more thought. My initial values here are all placeholders.

@coriolinus
Copy link
Contributor Author

@kianenigma Aside from the SlashHandler, all those values have now been changed.

@kianenigma
Copy link
Contributor

@kianenigma Aside from the SlashHandler, all those values have now been changed.

feedback given in other mediums; needs more tweaks.

@coriolinus
Copy link
Contributor Author

@kianenigma I worked out the function we walked about for computing an appropriate value for the signed rewards, which would pay back 1.5x the submission fee. It's implemented here. However, it doesn't work, because it needs to be a const fn to be legal within the parameter_types macro, but it can't be a const fn because it depends on trait functions. I therefore reverted it.

@kianenigma
Copy link
Contributor

@kianenigma I worked out the function we walked about for computing an appropriate value for the signed rewards, which would pay back 1.5x the submission fee. It's implemented here. However, it doesn't work, because it needs to be a const fn to be legal within the parameter_types macro, but it can't be a const fn because it depends on trait functions. I therefore reverted it.

remove const from parameter_types and it will accept non const values as well.

@coriolinus coriolinus force-pushed the prgn-companion-for-7910 branch from 07e9101 to ff228bc Compare June 24, 2021 06:58
@coriolinus
Copy link
Contributor Author

coriolinus commented Jun 24, 2021 •
edited
Loading

🤦 Welp, I should have thought of that. For some reason I thought that parameter_types required const values, despite counterexamples just a few lines away.

Ok, as of ff228bc, SignedRewardBase is 1.5x the submission fee for the extrinsic. This is an interesting value, because it means that for more than 1 miner, the expected value of any particular submission goes negative. (The miners will get their deposit back, but they'll still have to pay the fee to submit the solution.) It'll generate a small profit for us as long as we're the only people running a signed solution miner, and anyone else considering running a miner is likely to notice that on average, a miner in a pool of 2 will receive a reward of 0.75x the submission fee.

[edit]: That expected value assumes that all miners submit a solution for each election. The calculation changes if miners only submit when they believe that they have a solution which will beat the current best.

We can work out the economics of decentralization later. For now, I hope that this is a sufficient starting point for this PR.

@coriolinus coriolinus added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jun 24, 2021
kianenigma
kianenigma reviewed Jun 24, 2021
View reviewed changes
runtime/common/src/elections.rs
WeightInfo: pallet_election_provider_multi_phase::WeightInfo,
{
let expected_weight = WeightInfo::submit(T::SignedMaxSubmissions::get());
multiplier * WeightToFee::calc(&expected_weight)
Copy link
Contributor

@kianenigma kianenigma Jun 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

this might lead to negative turnout to run the bot, since your discounting the length fee (which in this case is probably more dominant than the weight fee)(I also forgot about length fee earlier).

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe what you should do is run your polkadot branch and submit a solution to it via the miner, to both obtain this and do an end-to-end test

might still help, but not a blocker since this is too conservattive and thus safe.

@ghost
Copy link

ghost commented Jun 28, 2021

Waiting for commit status.

@ghost
Copy link

ghost commented Jun 28, 2021

Merge aborted: Checks failed for 7084efc

@ordian ordian added the D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited. label Jun 28, 2021
@coriolinus coriolinus mentioned this pull request Jun 28, 2021
Merged
@ordian ordian merged commit d5fb327 into master Jun 28, 2021
@ordian ordian deleted the prgn-companion-for-7910 branch June 28, 2021 12:36
ordian added a commit that referenced this pull request Jul 2, 2021
@ordian
Merge branch 'master' into ao-dbg-unresponsiveness
efe630b 
* master: (21 commits)
  cleanup stream polls (#3397)
  Staking Miner (#3141)
  Companion for Substrate#8953 (#3140)
  Bump version, specs & substrate in prep for v0.9.8 (#3387)
  Fix busy loops. (#3392)
  Minor refactor (#3386)
  add simnet tests (#3381)
  BEEFY: adjust gossip (#3372)
  Companion for #9193 (#3376)
  Companion for Decouple Staking and Election - Part 3: Signed Phase (#2793)
  Ensure that we fetch another collation if the first collation was invalid (#3362)
  Only send one collation per relay parent at a time to validators (#3360)
  disable approval-checking-grandpa on dev chain (#3364)
  Use associated constant for max (#3375)
  Use wasm-builder from git (#3354)
  Squashed 'bridges/' changes from b2099c5..23dda62 (#3369)
  Bump versions & spec_versions (#3368)
  Don't allow bids for a ParaId where there is an overlapping lease period (#3361)
  Companion for upgrade of transaction-payment to pallet macro (#3267)
  Do not allow any crowdloan contributions during the VRF period (#3346)
  ...
@github-actions github-actions bot mentioned this pull request Jul 8, 2021
Closed
@viniul viniul added D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited. labels Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kianenigma kianenigma kianenigma approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@coriolinus @kianenigma @ordian @viniul