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

Constitutionally Limited: Impose certain additional limits on governance actions #582

Merged
merged 4 commits into from
Apr 21, 2020

Conversation

Shadowfiend
Copy link
Contributor

@Shadowfiend Shadowfiend commented Apr 18, 2020

Three limitations on governance actions:

  • Prevents lot sizes less than 0.0005 BTC.
  • Prevents lot sizes greater than 10 BTC.
  • Prevents signer fees below 5bps (0.05%).

Still needed:

  • Tests.

Closes #552 .

@Shadowfiend Shadowfiend changed the title Constitutionally Limited: Constitutionally Limited: Impose certain additional limits on governance actions Apr 18, 2020
This to avoid the ability to disable the system by simply zeroing out
the signer fee through governance.
This to avoid signer DoS (for the minimum 0.0005 BTC lot size) and to
avoid putting more funds at risk than reasonable in a single deposit
(for the maximum 10 BTC lot size).
@Shadowfiend Shadowfiend marked this pull request as ready for review April 19, 2020 01:06
@Shadowfiend Shadowfiend requested a review from a team April 19, 2020 01:06
@Shadowfiend
Copy link
Contributor Author

@liamzebedee @NicholasDotSol this is ready for one of you to have a look at.

);
require(
_signerFeeDivisor < 2000,
"Signer fee divisor must be less than 2000, for a signer fee that is > 5bps."
Copy link
Contributor

@liamzebedee liamzebedee Apr 20, 2020

Choose a reason for hiding this comment

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

Perhaps > 0.005% instead of quoting bps? We use % fairly consistently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That’s fair, yeah.

Copy link
Contributor

@liamzebedee liamzebedee left a comment

Choose a reason for hiding this comment

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

No blockers here.

Specifically, the revert message was in basis points while the one for
the upper bound was in percent. Align on percentages, which are used in
several other places as well.
@Shadowfiend Shadowfiend merged commit 93d4dae into master Apr 21, 2020
@Shadowfiend Shadowfiend deleted the constitutionally-limited branch April 21, 2020 00:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remaining limitations for deposit parameter updates
2 participants