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

Quartermaster #6

Closed
wants to merge 4 commits into from
Closed

Quartermaster #6

wants to merge 4 commits into from

Conversation

plor
Copy link

@plor plor commented Nov 11, 2023

This is a new shaman borrowing heavily from the onboarder shaman in this repository. It's intention is to give the captain (address holding the captain hat) the ability to bring new members to a DAO (adding them to the crew). Each crew member receives the minimum shares. In addition there is an onboarding delay so that if members notice the captain is making poor decisions in bringing on new members they can either ragequit or mutiny. The delay is twice the voting period to allow for this action.

I'm happy to work with the team to figure out how best to approach this. If there is another approach to introducing this module please let me know.

@spengrah spengrah self-requested a review November 12, 2023 18:00
Copy link
Member

@spengrah spengrah left a comment

Choose a reason for hiding this comment

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

I would actually encourage you to own this contract in your own repo. You can have it included in the list of Hats-powered tools in the Hats Community Wiki.

Note that even though it inherits from HatsModule, it doesn't count to be included in the Hats Modules Registry since it is neither an eligibility module, toggle module, or hatter contract. The wiki page referenced above is an early version of a more general "awesome list".

}

// OWNER_HAT is renamed to CAPTAIN_HAT for this use
// TODO I think this is wrong, this is brought from parent, captain should be initarg
Copy link
Member

Choose a reason for hiding this comment

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

Are you still considering moving this to mutable storage configured via init args? I think the immutable arg approach makes sense for this value.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I'll remove that TODO, it was from before I understood how the args were working.

/**
* @notice Adds votingPeriod x2 to the current time to allow for mutiny delay
*/
function _calculateDelay() private view returns (uint256 delay) {
Copy link
Member

Choose a reason for hiding this comment

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

If the Captain is a member of the DAO with enough votes to sponsor a proposal, they can accomplish the same thing as onboarding with a similar trust module but more directly by creating a proposal to add the member of their choice.

I would actually consider decreasing the delay to differentiate the onboarding pathway even more.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, that's how I'm accomplishing it without this. My main goal is to give the captain hat unilateral control over DAO operations. So bringing crew aboard without votes is the intention here. I think the delay is needed to avoid sybil or collusion attacks by the captain.

Copy link
Member

Choose a reason for hiding this comment

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

I guess my point is that giving a captain unilateral control presumably comes with some degree of trust anyways. Or another way to say it is that if you don't trust the captain enough to not require such a long delay, perhaps they shouldn't be the captain. But perhaps I'm misunderstanding your intention here.

* @title Quartermaster Shaman
* @notice A Baal manager shaman that allows onboarding, offboarding, and other DAO member management
* by the holder of the captain hat. The captain uses the quartermaster to give crew status to new members,
* but there is a delay to avoid the captain gathering crew to avoid a mutiny.
Copy link
Member

Choose a reason for hiding this comment

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

What is a mutiny?

Copy link
Author

Choose a reason for hiding this comment

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

mutiny is just a proposal that transfers the captain hat to someone else. the crew needs the delay to allow for time to mutiny if the captain is up to any shenanigans.

Copy link
Member

Choose a reason for hiding this comment

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

ah, nice!

@plor
Copy link
Author

plor commented Nov 12, 2023

Cool, I can keep it there. I borrowed heavily from the onboarder, so thought it would be good to at least offer it up, but because it is pretty specific to this use-case it makes sense to not include it in this repo. Thanks for checking it out though. I'll be sure to add it to the wiki when it's ready.

@spengrah
Copy link
Member

Cool, I can keep it there. I borrowed heavily from the onboarder, so thought it would be good to at least offer it up, but because it is pretty specific to this use-case it makes sense to not include it in this repo. Thanks for checking it out though. I'll be sure to add it to the wiki when it's ready.

@plor To be clear, the suggestion is less about restricting this contract from this repo, and more about wanting to highlight the work of ecosystem developers. Developers controlling their own repos is more in line with a robust ecosystem of modular hats-powered tools.

I should also have mentioned that you should definitely submit this as a contribution to the hats protoDAO.

@plor
Copy link
Author

plor commented Nov 12, 2023

Thanks, will do!

@plor plor closed this Nov 12, 2023
@plor plor deleted the quartermaster branch November 13, 2023 18:23
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.

2 participants