feat!(Contracts): assert success if Execute.success_required (#30)

Author: fubuloubu

contracts/Caravan.vy

@@ -26,10 +26,10 @@ MODIFY_TYPEHASH: constant(bytes32) = keccak256(
 )
 
 CALL_TYPEHASH: constant(bytes32) = keccak256(
-    "Call(address target,uint256 value,bytes data)"
+    "Call(address target,uint256 value,bool success_required,bytes data)"
 )
 EXECUTE_TYPEHASH: constant(bytes32) = keccak256(
-    "Execute(bytes32 parent,Call[] calls)Call(address target,uint256 value,bytes data)"
+    "Execute(bytes32 parent,Call[] calls)Call(address target,uint256 value,bool success_required,bytes data)"
 )
 
 # @dev The current implementation address for `CaravanProxy`
@@ -294,6 +294,7 @@ def execute(
                         CALL_TYPEHASH,
                         call.target,
                         call.value,
+                        call.success_required,
                         # NOTE: Per EIP712, Dynamic ABI types are encoded as the hash of their contents
                         keccak256(call.data),
                     )
@@ -321,12 +322,19 @@ def execute(
             extcall guard.preExecuteCheck(call)
 
         # NOTE: No delegatecalls allowed (cannot modify configuration via `update` this way)
-        success: bool = raw_call(
-            call.target,
-            call.data,
-            value=call.value,
-            revert_on_failure=False,
-        )
+        success: bool = True
+        if call.success_required:
+            # NOTE: Allow failure to bubble up normally
+            raw_call(call.target, call.data, value=call.value)
+
+        else:
+            success = raw_call(
+                call.target,
+                call.data,
+                value=call.value,
+                revert_on_failure=False,
+            )
+
 
         if guard.address != empty(address):
             extcall guard.postExecuteCheck()

contracts/ICaravan.vyi

@@ -2,6 +2,7 @@
 struct Call:
     target: address
     value: uint256
+    success_required: bool
     # TODO: Increase size to 65540 once Vyper improves memory allocation (gas costs too high)
     data: Bytes[16388]
 

pyproject.toml

@@ -10,6 +10,7 @@ readme = "README.md"
 requires-python = ">=3.11"
 dependencies = [
     "eth-ape>=0.8.43",
+    "eip712>=0.3.3",  # NOTE: Special pin needed
     "createx>=0.1.1",
 ]
 
@@ -20,6 +21,7 @@ test = [
     "ape-vyper>=0.8.9",
 ]
 dev = [
+    "ape-etherscan>=0.8.5",
     {include-group = "test"},
 ]
 
@@ -45,17 +47,3 @@ name = "foundry"
 name = "solidity"
 [[tool.ape.plugins]]
 name = "vyper"
-
-# Factory
-#[[tool.ape.deployments.ethereum.deterministic]]
-[[tool.ape.deployments.ethereum.local]]
-address = "0x04579FFC45fE10A7901B88EaEc8F4850b847D37c"
-contract_type = "CaravanFactory"
-
-# Singleton
-# NOTE: Update latest Singleton when contract code changes
-# v1 (Latest)
-#[[tool.ape.deployments.ethereum.deterministic]]
-[[tool.ape.deployments.ethereum.local]]
-address = "0x685B0DEA9daF4DC73Ab9dba4bc4156d90A7F8a5d"
-contract_type = "Caravan"

sdk/py/caravan/factory.py

@@ -18,17 +18,24 @@ def __init__(self, address: AddressType | None = None):
             self.address = address
 
         elif len((factory_type := PackageType.FACTORY()).deployments) == 0:
-            raise RuntimeError("No CaravanFactory deployment on this chain")
+            # NOTE: This is the deterministic deployment address via CreateX
+            self.address = "0x04579FFC45fE10A7901B88EaEc8F4850b847D37c"
+
+            if not len(self.provider.get_code(self.address)) > 0:
+                raise RuntimeError("No CaravanFactory deployment on this chain")
 
         else:
             # NOTE: Override cached value of `contract`
             self.contract = factory_type.deployments[0]
             self.address = self.contract.address
 
         # NOTE: also lets us override for testing
-        self._cached_releases: dict[Version, "ContractInstance"] = dict()
-
-    # TODO: classmethod `.inject`?
+        self._cached_releases: dict[Version, "ContractInstance"] = {
+            # NOTE: This is the deterministic deployment address for v1 via CreateX
+            Version("1"): PackageType.SINGLETON("1").at(
+                "0xf7AC37e8A31Da4D2Fbe7687118c142dd46e63517"
+            ),
+        }
 
     @cached_property
     def contract(self) -> "ContractInstance":
@@ -39,13 +46,13 @@ def contract(self) -> "ContractInstance":
         )
 
     def get_release(self, version: Version) -> "ContractInstance | None":
-        if release := self._cached_releases.get(version):
+        if (release := self._cached_releases.get(version)) and len(release.code) > 0:
             return release
 
         elif not (singleton_type := PackageType.SINGLETON(version)).deployments:
             return None
 
-        elif not (release := singleton_type.deployments[-1]).code:
+        elif not len((release := singleton_type.deployments[-1]).code) > 0:
             return None
 
         self._cached_releases[version] = release
@@ -65,7 +72,8 @@ def new(
         if isinstance(version, str):
             version = Version(version.lstrip("v"))
 
-        release = self.get_release(version)
+        if not (release := self.get_release(version)):
+            raise RuntimeError("No Caravan Singleton deployment on this chain")
 
         args = [release, signers, threshold]
         if tag is not None:

sdk/py/caravan/messages/execute.py

@@ -10,6 +10,7 @@
 
 if TYPE_CHECKING:
     from ape.api import ReceiptAPI
+    from ape.api.address import BaseAddress
     from ape.types import AddressType
     from packaging.version import Version
 
@@ -20,10 +21,11 @@
 class Call(BaseModel):
     target: abi.address
     value: abi.uint256
+    success_required: bool
     data: HexBytes
 
     def render(self) -> str:
-        return f"{self.target}({self.data}, value={self.value})"
+        return f"{self.target}({self.data}, value={self.value}, required={self.success_required})"
 
 
 class Execute(EIP712Message, ManagerAccessMixin):
@@ -72,7 +74,11 @@ def new(
         return self
 
     def add_raw(
-        self, target: "AddressType", value: int = 0, data: HexBytes = b""
+        self,
+        target: "BaseAddress | AddressType | str",
+        value: str | int = 0,
+        success_required: bool = True,
+        data: HexBytes = b"",
     ) -> Self:
         if len(self.calls) >= self.MAX_CALLS:
             raise RuntimeError(
@@ -85,20 +91,48 @@ def add_raw(
                 f" {self.MAX_CALLDATA_SIZE} bytes."
             )
 
-        self.calls.append(Call(target=target, value=value, data=data))
+        # NOTE: Avoid imports otherwise just for typing
+        from ape.types import AddressType
+
+        self.calls.append(
+            Call(
+                target=self.conversion_manager.convert(target, AddressType),
+                value=self.conversion_manager.convert(value, int),
+                success_required=success_required,
+                data=data,
+            )
+        )
         return self
 
-    def add(self, call, *args, value: int = 0) -> Self:
+    def add(self, call, *args, value: int = 0, success_required: bool = True) -> Self:
         return self.add_raw(
-            target=call.contract.address,
+            target=call.contract,
             value=value,
+            success_required=success_required,
             data=call.encode_input(*args),
         )
 
-    def add_from_receipt(self, receipt: "ReceiptAPI") -> Self:
+    def add_transfer(
+        self,
+        target: "BaseAddress | AddressType | str",
+        value: int | str,
+        data: bytes = b"",
+        success_required: bool = True,
+    ):
+        return self.add_raw(
+            target=target,
+            value=value,
+            success_required=success_required,
+            data=data,  # In case you want to add a message or something
+        )
+
+    def add_from_receipt(
+        self, receipt: "ReceiptAPI", success_required: bool = True
+    ) -> Self:
         return self.add_raw(
             target=receipt.receiver,
             value=receipt.value,
+            success_required=success_required,
             data=receipt.data,
         )
 

tests/test_operation.py

@@ -37,32 +37,51 @@ def test_initialize(singleton, van, THRESHOLD, owners):
 def test_execute(accounts, van, owners, calls):
     msg = van.new_batch()
 
+    accounts[-1].transfer(van.contract, "10 ether")
+    starting_balance = {van.contract.address: van.contract.balance}
     for idx in range(total_calls := int(calls.split("_")[0])):
-        msg.add_raw(
-            target=accounts[idx].address,
-            value=idx,
+        msg.add_transfer(
+            target=accounts[idx],
+            value=f"{idx + 1} ether",
             data=f"{idx}".encode("utf-8"),
         )
 
+        # NOTE: track for later assertion
+        starting_balance[a.address] = (a := accounts[idx]).balance
+
     assert van.head == msg.parent
     assert msg not in van.queue
 
     van.stage(msg)
     assert msg in van.queue
 
     receipt = van.commit(msg, sender=owners[0])
+    if total_calls > 0:
+        # NOTE: We need this to ensure accurate tracking for later in `sum()`
+        starting_balance[owners[0].address] -= receipt.gas_used * receipt.gas_price
     assert van.head == msg.hash
 
     if total_calls > 0:
         assert receipt.events == [
             van.contract.Executed(
                 executor=owners[0],
                 target=account,
-                value=idx,
+                value=(idx + 1) * int(1e18),
                 data=f"{idx}".encode("utf-8"),
             )
             for idx, account in enumerate(accounts[:total_calls])
         ]
+        assert all(
+            a.balance == starting_balance[a.address] + (idx + 1) * int(1e18)
+            for idx, a in enumerate(accounts[:total_calls])
+        )
+        assert van.contract.balance == (
+            starting_balance[van.contract.address]
+            - sum(
+                a.balance - starting_balance[a.address] for a in accounts[:total_calls]
+            )
+        )
 
     else:
         assert receipt.events == []
+        assert van.contract.balance == starting_balance[van.contract.address]