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

Safe Harbor Agreement Implementation Standard #5

Merged
merged 79 commits into from
Sep 30, 2024

Conversation

Robert-MacWha
Copy link
Contributor

@Robert-MacWha Robert-MacWha commented Mar 21, 2024

EDIT: 2024-06-23

Marking this PR as functionally complete. Changes to form may be performed, but avoid making functional changes barring catastrophe.

Original Message

Following up with Discussion #4, I created this document as a V1 of the Safe Harbor Agreement Implementation Standard. It outlines a standardization for the contents of AgreementDetails, ensuring consistency and compliance across all agreements. Standardization of these contents will help Whitehats understand the contents of the Agreement more easily in potentially time-sensitive environments, and will help software parse and display multiple agreements in a standardized manner.

It outlines specific standards in JSON format for the scope, contactDetails and bountyTerms fields of the AgreementDetails struct, providing structured information regarding static assets, implicit assets, primary and secondary contacts, identity requirements, diligence requirements, bounty percentage, cap, and retainability; and sections for additional information where necessary.

@rholterhus
Copy link
Contributor

Amazing, thanks for putting this together. The structure looks good to me, and one interesting thing I hadn't considered before is your implicitAssets idea (in the scope field) where you can specify a deployer address instead of a fixed set of addresses. This seems like a reasonable choice for protocols that use a factory contract to deploy things.

Before merging the PR I'm going to do two things:

  1. Double-check whether the on-chain component or the IPFS component should take precedence legally (I forget the answer). Depending on the answer it might require a quick change here:

    In the event of any conflict or inconsistency between information in the AgreementDetails and the text of the Agreement, the text of the Agreement will govern

  2. Will check if we should wait to officially merge this once we have a few more incoming changes ready (working on some reorganizing and other small tweaks)

Thanks again and will circle back here

@Robert-MacWha
Copy link
Contributor Author

  1. Double-check whether the on-chain component or the IPFS component should take precedence legally (I forget the answer). Depending on the answer it might require a quick change here:

Should have linked to the Agreement — I got this from EXHIBIT F (Adoption Form) where it states “In the event of any conflict or inconsistency between the summary and the text of the Agreement, the text of the Agreement will govern.” Sounded like the text of the Agreement is the source of truth, so I included similar language.

Probably good idea to double-check.

Adjust names of `direct` and `child` assets.

Clarify contact information.

Generally simplify descriptions.
@Robert-MacWha
Copy link
Contributor Author

I was making the website form, and it occurred to me that direct and child are probably better terms that static and implicity.

Updated the PR - if you have any objections let me know.

@Robert-MacWha
Copy link
Contributor Author

Robert-MacWha commented Jun 19, 2024

RE: The conversion I had with Gabe.

Updated the smart contracts to have upgradable agreements & to encode the implementation standard in code. In short, adopting the agreement now requires deploying a smart contract that will record adoption details from a pre-approved contract deployed.

Adoption changes
Adoption requires calling the adoptSafeHarbor method on the AgreementDetailDeployerV1 contract while providing the adoption-specific AgreementDetailsV1. Calling adoptSafeHarbor method will:

  1. Deploy a AgreementV1 contract that records the AgreementDetailsV1 on-chain
  2. Call recordAdoption on the main registry contract, which will in turn record the adoption in the registry's mapping and emit an SafeHarborAdoption event.

Setup Changes
Initial setup of the registry is now more involved. In addition to deploying the main registry contract (which requires an admin account) a AgreementDetailDeployer must also be deployed and approved using the registry contract's approveDeployer method.

AdoptionDetails Changes
The structure of the AgreementDetails struct has again been changed to increase specificity. The new structure mirrors that within the legal contracts.

struct AgreementDetailsV1 {
    // The name of the protocol adopting the agreement.
    string protocolName;
    // The accounts in scope of the agreement.
    Account[] scope;
    // The contact details (required for pre-notifying).
    Contact[] contactDetails;
    // The bounty terms.
    BountyTerms bountyTerms;
    // Indication whether the agreement should be automatically upgraded to future versions approved by SEAL.
    bool automaticallyUpgrade;
    // Address where recovered funds should be sent.
    address assetRecoveryAddress;
    // IPFS hash of the actual agreement document, which confirms all terms.
    string agreementURI;
}

struct Account {
    // The address of the account (EOA or smart contract).
    address accountAddress;
    // The chain IDs on which the account is present.
    uint[] chainIDs;
    // Whether smart contracts deployed by this account are in scope.
    bool includeChildContracts;
    // Whether smart contracts deployed by this account after the agreement is adopted are in scope.
    bool includeNewChildContracts;
}

struct Contact {
    // The name of the contact.
    string name;
    // The role of the contact.
    string role;
    // The contact details (IE email, phone, telegram).
    string contact;
}

enum IdentityRequirement {
    Anonymous,
    Pseudonymous,
    Named
}

struct BountyTerms {
    // Percentage of the recovered funds a Whitehat receives as their bounty (0-100).
    uint bountyPercentage;
    // The maximum bounty in USD.
    uint bountyCapUSD;
    // Indicates if the Whitehat can retain their bounty.
    bool retainable;
    // Identity requirements for Whitehats eligible under the agreement.
    IdentityRequirement identityRequirement;
    // Description of what KYC, sanctions, diligence, or other verification will be performed on Whitehats to determine their eligibility to receive the bounty.
    string diligenceRequirements;
}

Signatures should be performed on encoded data, not on the raw hash.  Added a new `encode` function that encodes a `AgreementDetails` and returns the hashed value.  Updated all tests and `GenerateAccountSignatureV1`

```
encode(domainSeparator : 𝔹²⁵⁶, message : 𝕊) = "\x19\x01" ‖ domainSeparator ‖ hashStruct(message)
```
Enums aren't really specified, so I don't want to deal with them.
@Robert-MacWha Robert-MacWha merged commit 768e350 into security-alliance:main Sep 30, 2024
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.

3 participants