-
Couldn't load subscription status.
- Fork 305
fix(hip-3-pusher): Multisig update support (only 1/N, no KMS) #3147
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Suggested a minor improvement but nothing that blocks merging
| if config.multisig.enable_multisig: | ||
| if not config.multisig.multisig_address: | ||
| raise Exception("Multisig enabled but missing multisig address") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can enforce this directly in the pydantic model. Doing as much validation as possible in the model itself aligns with pydantic's philosophy of "if i can construct the object, i know it's valid."
class MultisigConfig(BaseModel):
enable_multisig: bool
multisig_address: Optional[str] = None
@model_validator(mode="after")
def validate_address_exists_if_multisig_enabled(self) -> 'MultisigConfig':
if self.enable_multisig and self.multisig_address is None:
raise ValueError("`multisig_address` must be provided when `enable_multisig` is True")
return self
Summary
Support multisig oracle update as per https://hyperliquid.gitbook.io/hyperliquid-docs/hypercore/multi-sig .
Rationale
This allows us to provide our own wallet for updating without ever having access to the deployer oracle account. They can grant and revoke access at will.
How has this been tested?