From c81e981394c3ea55b08f16045c8d5b3fb8ed53b4 Mon Sep 17 00:00:00 2001 From: Alain Nicolas Date: Tue, 26 Nov 2024 10:58:20 +0100 Subject: [PATCH] chore(contracts): Document potential accounting issue with Module --- contracts/src/abstracts/AbstractPortal.sol | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/contracts/src/abstracts/AbstractPortal.sol b/contracts/src/abstracts/AbstractPortal.sol index 388a283c..0bbedaee 100644 --- a/contracts/src/abstracts/AbstractPortal.sol +++ b/contracts/src/abstracts/AbstractPortal.sol @@ -61,6 +61,8 @@ abstract contract AbstractPortal is IPortal { * @param attestationPayload the payload to attest * @param validationPayloads the payloads to validate via the modules to issue the attestations * @dev Runs all modules for the portal and registers the attestation using AttestationRegistry + * @dev WARNING: Ensure that at most one module processes `msg.value` to avoid accounting issues, + * as the total `msg.value` is forwarded to all modules. */ function attest(AttestationPayload memory attestationPayload, bytes[] memory validationPayloads) public payable { moduleRegistry.runModules(modules, attestationPayload, validationPayloads, msg.value); @@ -75,6 +77,8 @@ abstract contract AbstractPortal is IPortal { * @param attestationPayload the payload to attest * @param validationPayloads the payloads to validate via the modules to issue the attestations * @dev Runs all modules for the portal and registers the attestation using AttestationRegistry + * @dev WARNING: Ensure that at most one module processes `msg.value` to avoid accounting issues, + * as the total `msg.value` is forwarded to all modules. */ function attestV2(AttestationPayload memory attestationPayload, bytes[] memory validationPayloads) public payable { moduleRegistry.runModulesV2( @@ -99,6 +103,8 @@ abstract contract AbstractPortal is IPortal { * @dev DISCLAIMER: This method may have unexpected behavior if one of the Module checks is done on the attestation ID * as this ID won't be incremented before the end of the transaction. * If you need to check the attestation ID, please use the `attest` method. + * @dev WARNING: Ensure that at most one module processes `msg.value` to avoid accounting issues, + * as the total `msg.value` is forwarded to all modules. */ function bulkAttest(AttestationPayload[] memory attestationsPayloads, bytes[][] memory validationPayloads) public { moduleRegistry.bulkRunModules(modules, attestationsPayloads, validationPayloads); @@ -115,6 +121,8 @@ abstract contract AbstractPortal is IPortal { * @dev DISCLAIMER: This method may have unexpected behavior if one of the Module checks is done on the attestation ID * as this ID won't be incremented before the end of the transaction. * If you need to check the attestation ID, please use the `attestV2` method. + * @dev WARNING: Ensure that at most one module processes `msg.value` to avoid accounting issues, + * as the total `msg.value` is forwarded to all modules. */ function bulkAttestV2(AttestationPayload[] memory attestationPayloads, bytes[][] memory validationPayloads) public { moduleRegistry.bulkRunModulesV2( @@ -137,6 +145,8 @@ abstract contract AbstractPortal is IPortal { * @param attestationPayload the attestation payload to create the new attestation and register it * @param validationPayloads the payloads to validate via the modules to issue the attestation * @dev Runs all modules for the portal and registers the attestation using AttestationRegistry + * @dev WARNING: Ensure that at most one module processes `msg.value` to avoid accounting issues, + * as the total `msg.value` is forwarded to all modules. */ function replace( bytes32 attestationId, @@ -156,6 +166,8 @@ abstract contract AbstractPortal is IPortal { * @param attestationPayload the attestation payload to create the new attestation and register it * @param validationPayloads the payloads to validate via the modules to issue the attestation * @dev Runs all modules for the portal and registers the attestation using AttestationRegistry + * @dev WARNING: Ensure that at most one module processes `msg.value` to avoid accounting issues, + * as the total `msg.value` is forwarded to all modules. */ function replaceV2( bytes32 attestationId, @@ -185,6 +197,8 @@ abstract contract AbstractPortal is IPortal { * @dev DISCLAIMER: This method may have unexpected behavior if one of the Module checks is done on the attestation ID * as this ID won't be incremented before the end of the transaction. * If you need to check the attestation ID, please use the `replace` method. + * @dev WARNING: Ensure that at most one module processes `msg.value` to avoid accounting issues, + * as the total `msg.value` is forwarded to all modules. */ function bulkReplace( bytes32[] memory attestationIds, @@ -206,6 +220,8 @@ abstract contract AbstractPortal is IPortal { * @dev DISCLAIMER: This method may have unexpected behavior if one of the Module checks is done on the attestation ID * as this ID won't be incremented before the end of the transaction. * If you need to check the attestation ID, please use the `replaceV2` method. + * @dev WARNING: Ensure that at most one module processes `msg.value` to avoid accounting issues, + * as the total `msg.value` is forwarded to all modules. */ function bulkReplaceV2( bytes32[] memory attestationIds, @@ -230,7 +246,7 @@ abstract contract AbstractPortal is IPortal { * @notice Revokes an attestation for the given identifier * @param attestationId the ID of the attestation to revoke * @dev By default, revocation is only possible by the portal owner - * We strongly encourage implementing such a rule in your Portal if you intend on overriding this method + * We strongly encourage implementing such a rule in your Portal if you intend on overriding this method */ function revoke(bytes32 attestationId) public { _onRevoke(attestationId);