CoopHive / alkahest-mocks

3 stars 0 forks source link

separating "obligation" statements from "comment" (currently validator) statements #2

Closed polus-arcticus closed 1 month ago

polus-arcticus commented 1 month ago

perhaps change https://github.com/CoopHive/tokens-for-docker-alkahest/blob/b7a79093684fe31863228f16306a285bf9db0e25/src/statements/DockerResultStatement.sol#L9 to

contract DockerResultStatement is IValidatorStatement and https://github.com/CoopHive/tokens-for-docker-alkahest/blob/b7a79093684fe31863228f16306a285bf9db0e25/src/statements/ERC20PaymentStatement.sol#L13 to contract ERC20PaymentStatement is IObligationStatement

where IValidateStatement and IObliationStatement are

abstract contract IValidateStatement is IStatement abstract contract IObligationStatement is IStatement

?

mlegls commented 1 month ago

Not sure if it's worth this much indirection. Can you think of fields that would go in IValidateStatement and IObligationStatement to differentiate them? Right now statement contracts seem pretty lightweight - they're just contracts that make attestations

mlegls commented 1 month ago

I'm considering something like

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.26;

import {Attestation} from "@eas/Common.sol";
import {IStatement} from "./IStatement.sol";
import {IEAS, AttestationRequest, AttestationRequestData, RevocationRequest, RevocationRequestData} from "@eas/IEAS.sol";

abstract contract IObligation is IStatement {
    error CantCancel();
    error UnauthorizedCall();
    error MakeFailed();

    bool public immutable CAN_CANCEL;

    function make(
        bytes calldata data,
        uint64 expirationTime,
        bytes32 fulfilling
    ) external returns (bytes32) {
        if (!_make(data, expirationTime, fulfilling)) revert MakeFailed();
        return
            eas.attest(
                AttestationRequest({
                    schema: ATTESTATION_SCHEMA,
                    data: AttestationRequestData({
                        recipient: msg.sender,
                        expirationTime: expirationTime,
                        revocable: true,
                        refUID: fulfilling,
                        data: data,
                        value: 0
                    })
                })
            );
    }

    function cancel(bytes32 uid) external returns (bool) {
        if (!CAN_CANCEL) revert CantCancel();
        Attestation memory attestation = eas.getAttestation(uid);
        if (msg.sender != attestation.recipient) revert UnauthorizedCall();
        eas.revoke(
            RevocationRequest({
                schema: ATTESTATION_SCHEMA,
                data: RevocationRequestData({uid: uid, value: 0})
            })
        );
        return _cancel(attestation);
    }

    function _make(
        bytes calldata data,
        uint64 expirationTime,
        bytes32 fulfilling
    ) internal virtual returns (bool);

    function _cancel(
        Attestation memory attestation
    ) internal virtual returns (bool);
}

Where the implementer ideally doesn't have to see the EAS details directly anymore. Would be really nice with generic types but it doesn't seem like they're coming to solidity anytime soon. This would mean the user-facing contracts have to take bytes input, which is pretty bad for UX.

Also, not sure if the _cancel hook should take an extra bytes parameter - so e.g. implementations could put all cancellation conditions together in one hook with a switch/case type dispatch. Maybe good for clarity but bad for performance, and kind of unintuitive to implement. Another possibility is that the cancel/_cancel in the abstract contract only represent "default" parameterless cancellation, and implementers would still have to explicitly import EAS stuff and do eas.revoke for custom cancellation conditions, like fee collection.

I still don't have a lot of clarity on the relationship between EAS's onAttest/onRevoke and these abstract contracts, and whether there would be any good patterns in overriding onAttest/onRevoke. I do want to stay opinionated though, so that contract structure is encouraged to be uniform, by making the proper thing easy and improper things hard.

Also considering the possibility of a toolchain for code generation/linting to enforce uniformity, which might be more powerful and pleasant than relying on base solidity language features. Something like alkahest new [obligation/comment/arbiter] and alkahest lint. I think this direction would mean the abstract contracts should be more lightweight and performance-focused, since there's less reliance on solidity's type system to ensure implementations are doing the right thing.

polus-arcticus commented 1 month ago

@mlegls Indeed doing abi.encode for the bytes, especially if dynamic sized types are involved are a hastle even in solidity let alone in js or python. Though yes, without Generic types, or perhaps a lot of permutations and combinations of overloaded functions, bytes calldata _data or bytes memory _data will be the way for all user facing functions, cancel included.

Yes, I like the idea of a codegen alot - accepting a json or yaml file as input. It could spit out a foundry template with deployment code, and something that registers it with a central alkahest registry of some sort. Some may be disappointed there isn't hardhat+web3.js or what not, but the majority will likely be fine in the current popular toolchain