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

New guarantee mechanism #117

Merged
merged 22 commits into from May 11, 2020
Merged

Conversation

mmyyrroonn
Copy link
Member

@mmyyrroonn mmyyrroonn commented Apr 26, 2020

Fix #95
Guarantor increasing his votes won't bring side-effect.

  • Questions
    • What will happen when several guarantors guarantee concurrently.
    • Guarantor can attack the blockchain by calling guarantee millions of times. Each time he just increases his vote with a minimal staking.

Fix #118

@badkk
Copy link
Member

badkk commented Apr 27, 2020

Fix #95

Guarantor increasing his votes won't bring side-effect.

  • Questions

    • What will happen when several guarantors guarantee concurrently.

    • Guarantor can attack the blockchain by calling guarantee millions of times. Each time he just increases his vote with a minimal staking.

  • The first question is about tx pool in substrate, I cannot give a certain answer but we can obey substrate's runtime module function rules, AFAIK, this is an atomic operation.
  • The second question is all about ddos. They can hardly do this cause they need to pay tx fee. So the 2nd question is not our problem.

cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
@badkk badkk changed the title New staking mechanism New guarantee mechanism Apr 27, 2020
@badkk
Copy link
Member

badkk commented Apr 27, 2020

Currently, we combine {add new guarantee, delete old guarantee} into A single guarantee action, however, once we wanna resolve #95 , this became confusing of code logic, maybe we can split into totally 2 functions to simplify the logic @myronfanqiu

@badkk
Copy link
Member

badkk commented Apr 27, 2020

Fix #95
Guarantor increasing his votes won't bring side-effect.

  • Questions

    • What will happen when several guarantors guarantee concurrently.
    • Guarantor can attack the blockchain by calling guarantee millions of times. Each time he just increases his vote with a minimal staking.

Added to the 1st question, it includes 2 scenarios:

  1. Concurrency happens in the same node: it will be sorted by tx pool, and if v's stake limit reached, later priority guarantee tx will be rejected;
  2. Concurrency happens in the different nodes: if the tx sync is not on time(for example, if G1->V1 comes later than G2->V1, but G2->V1 already be accepted in another node's block); this depends on tx sync speed. If this happens(low priority), 1 of these 2 txs will be illegal(the tx included into final main-chain will be valid, at most 12s? in our system).

@badkk badkk added betanet A2-breakapi PR which break the outter api A0-breakconsensus PR which break the consensus, make blocking error A4-enhancement PR for refactor, better api or improve performance and removed A0-breakconsensus PR which break the consensus, make blocking error labels Apr 27, 2020
@badkk
Copy link
Member

badkk commented Apr 28, 2020

Talked offline, basically, this PR will make guarantee function into 2 separated functions:

  1. guarantee: guarantee new stake to validator;
  2. cut_guarantee: cut stake to current guarantee;

cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
@mmyyrroonn mmyyrroonn requested a review from badkk May 8, 2020 14:13
badkk
badkk previously approved these changes May 9, 2020
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
cstrml/staking/src/lib.rs Outdated Show resolved Hide resolved
@badkk badkk merged commit ce8408d into crustio:master May 11, 2020
@mmyyrroonn mmyyrroonn deleted the fix-95-multi-staking branch October 20, 2020 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A2-breakapi PR which break the outter api A4-enhancement PR for refactor, better api or improve performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GPoS] error message is unfriendly [GPoS] Guarantor increase his vote could bring a ranking side-effect
3 participants