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

Teams - Phase 1 #861

Open
wants to merge 76 commits into
base: main
Choose a base branch
from
Open

Conversation

0o-de-lally
Copy link
Collaborator

@0o-de-lally 0o-de-lally commented Nov 25, 2021

Implements delegation for consensus. Makes Carpe mining part of the security model, turning validators into DAOs

  • Move module Teams: creates team structure on a "captains" address. Captain is a validator.
  • Move tx scripts: Create Team, Join Team
  • Move Subsidy module, calculates reward split for captain and members of teams.
  • Functional tests for reward split, creating, joining teams.
  • TXS cli tool to include subcommands to create-team, and join-team
  • TXS lib bindings for Carpe to import to issue txs.
  • CLI lib binding to query for team struct for Carpe
  • Types: Rust mirror resource type for the Move Teams type.
  • Closes loophole with Validators removing self from candidate-set.
  • Corrects issue with epoch burn not being implemented in some cases.
  • Documentation

@0o-de-lally
Copy link
Collaborator Author

// Used for coordinating "Teams", equivalent to delegation
// Teams are groups which engage in work.
// some of that work is automated such as validation.
// A Team has a collective "consensus weight", which is used for voting in consensus
Copy link
Contributor

@sm86 sm86 Feb 16, 2022

Choose a reason for hiding this comment

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

know it might be late but just a thought: should we use consensus weight only for being in top 100 and consider all teams to be equal consensus weight. theoretically there is a probability of malicious node having higher consensus weight

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is interesting. Something for @zmanian

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sm86 Can you ticket this as an idea to research in Issues, for Phase 2?

Copy link
Contributor

@sm86 sm86 left a comment

Choose a reason for hiding this comment

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

looks good to me.

language/diem-framework/modules/0L/TowerState.move Outdated Show resolved Hide resolved
ol/txs/src/commands/create_team_cmd.rs Outdated Show resolved Hide resolved
language/diem-framework/modules/0L/Subsidy.move Outdated Show resolved Hide resolved
Copy link
Contributor

@soaresa soaresa left a comment

Choose a reason for hiding this comment

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

Congrats! Let's start this game

language/diem-framework/modules/0L/Teams.move Show resolved Hide resolved

// bob wants to switch to a different Team.
if (exists<Member>(addr)) {
let member_state = borrow_global_mut<Member>(addr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to check if the account is in the member list of the old team and if so, remove them

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch, working on it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed with 7dd61b6

Added switch_team private function.
Includes the switch_team.move test in functional test.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think switch_team would allow a user to be added a team without having first met the mining threshold that is enforced is maybe_activate_member _to_team. Perhaps maybe_switch_team should call maybe_activate_member_to_team in order to prevent this?

Copy link
Collaborator Author

@0o-de-lally 0o-de-lally Feb 21, 2022

Choose a reason for hiding this comment

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

@jamesmeijers please check, I refactored as you suggested. See: 5688b6a

@0o-de-lally 0o-de-lally changed the base branch from main-old-20220213 to main February 17, 2022 17:27
// needs to check if this is a slow wallet.
// ask user to resubmit if not a slow wallet, so they are explicitly setting it, no surprises, no tears.

assert(DiemAccount::is_slow(addr), ENOT_SLOW_WALLET);
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be a good idea to wait at least 1 epoch for a new team be able to receive new members because it creates anticipation for Carpe users, who can see new captains donation plans, and avoids validators to create their teams with few members near by the end of an epoch.

@sm86 sm86 self-requested a review February 18, 2022 01:25
Copy link
Contributor

@agouin agouin left a comment

Choose a reason for hiding this comment

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

Excited for this! LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants