fix: bind Tempo attribution memos to challenge IDs (#111)

* fix: bind Tempo attribution memos to challenge IDs

* chore: add changelog

* fix: normalize empty memos and defer hash replay marking

* fix: record verified transaction hashes in replay store

* Delete .changelog/brisk-wolves-track.md

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Author: brendanjryan

.changelog/quiet-birds-swim.md

@@ -0,0 +1,5 @@
+---
+pympp: patch
+---
+
+Fixed Tempo attribution memos to be deterministically bound to challenge IDs using a keccak256-derived nonce instead of random bytes. Added `verify_challenge_binding` to enforce that verified payments carry a memo matching the specific challenge being redeemed.

src/mpp/methods/tempo/_attribution.py

@@ -12,12 +12,11 @@
 | 4      | 1    | version (0x01)                            |
 | 5..14  | 10   | serverId = keccak256(serverId)[0..9]       |
 | 15..24 | 10   | clientId = keccak256(clientId)[0..9] or 0s |
-| 25..31 | 7    | nonce (random bytes)                      |
+| 25..31 | 7    | nonce = keccak256(challengeId)[0..6]       |
 """
 
 from __future__ import annotations
 
-import os
 from dataclasses import dataclass
 
 _VERSION = 0x01
@@ -43,21 +42,25 @@ def _fingerprint(value: str) -> bytes:
     return _keccak(value.encode())[:10]
 
 
+def challenge_nonce(challenge_id: str) -> bytes:
+    return _keccak(challenge_id.encode())[:7]
+
+
 def __getattr__(name: str):  # type: ignore[reportReturnType]
     if name == "TAG":
         return _get_tag()
     raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
 
 
-def encode(server_id: str, client_id: str | None = None) -> str:
+def encode(challenge_id: str, server_id: str, client_id: str | None = None) -> str:
     tag = _get_tag()
     buf = bytearray(32)
     buf[0:4] = tag
     buf[4] = _VERSION
     buf[5:15] = _fingerprint(server_id)
     if client_id:
         buf[15:25] = _fingerprint(client_id)
-    buf[25:32] = os.urandom(7)
+    buf[25:32] = challenge_nonce(challenge_id)
     return "0x" + buf.hex()
 
 
@@ -82,6 +85,16 @@ def verify_server(memo: str, server_id: str) -> bool:
     return memo_server == _fingerprint(server_id)
 
 
+def verify_challenge_binding(memo: str, challenge_id: str) -> bool:
+    if not is_mpp_memo(memo):
+        return False
+    try:
+        memo_nonce = bytes.fromhex(memo[52:66])
+    except ValueError:
+        return False
+    return memo_nonce == challenge_nonce(challenge_id)
+
+
 @dataclass(frozen=True, slots=True)
 class DecodedMemo:
     version: int

src/mpp/methods/tempo/client.py

@@ -116,8 +116,14 @@ async def create_credential(self, challenge: Challenge) -> Credential:
                 nonce_key = int(nonce_key)
 
         memo = method_details.get("memo") if isinstance(method_details, dict) else None
+        if memo == "":
+            memo = None
         if memo is None:
-            memo = encode_attribution(server_id=challenge.realm, client_id=self.client_id)
+            memo = encode_attribution(
+                challenge_id=challenge.id,
+                server_id=challenge.realm,
+                client_id=self.client_id,
+            )
 
         # Resolve RPC URL from challenge's chainId (like mppx), falling back
         # to the method-level rpc_url.

src/mpp/methods/tempo/intents.py

@@ -7,8 +7,9 @@
 
 import asyncio
 import time
+from dataclasses import dataclass
 from datetime import UTC, datetime
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING, Any, Literal
 
 import attrs
 
@@ -44,6 +45,12 @@
 ZERO_ADDRESS = "0x0000000000000000000000000000000000000000"
 
 
+@dataclass(frozen=True, slots=True)
+class MatchedTransferLog:
+    kind: Literal["memo", "transfer"]
+    memo: str | None = None
+
+
 def _rpc_error_msg(result: dict) -> str:
     """Extract error message from a JSON-RPC error response."""
     error_obj = result["error"]
@@ -230,21 +237,23 @@ async def verify(
             raise VerificationError(f"Invalid credential type: {payload_data['type']}")
 
         if isinstance(payload, HashCredentialPayload):
-            return await self._verify_hash(payload, req)
+            return await self._verify_hash(
+                payload,
+                req,
+                challenge_id=credential.challenge.id,
+                realm=credential.challenge.realm,
+            )
         else:
             return await self._verify_transaction(payload, req)
 
     async def _verify_hash(
         self,
         payload: HashCredentialPayload,
         request: ChargeRequest,
+        challenge_id: str,
+        realm: str,
     ) -> Receipt:
         """Verify a credential with a transaction hash."""
-        if self._store is not None:
-            store_key = f"mpp:charge:{payload.hash.lower()}"
-            if not await self._store.put_if_absent(store_key, payload.hash):
-                raise VerificationError("Transaction hash already used")
-
         client = await self._get_client()
 
         rpc_url = self._get_rpc_url()
@@ -270,19 +279,56 @@ async def _verify_hash(
         if receipt_data.get("status") != "0x1":
             raise VerificationError("Transaction reverted")
 
-        if not self._verify_transfer_logs(receipt_data, request):
+        matched_logs = self._verify_transfer_logs(receipt_data, request)
+        if not matched_logs:
             raise VerificationError(
                 "Transaction must contain a Transfer log matching request parameters"
             )
 
+        # Only verify challenge binding when using auto-generated attribution memos.
+        # Explicit memos (set by the server) are strictly matched by _verify_transfer_logs
+        # but are NOT challenge-bound. Callers that set explicit memos are responsible
+        # for ensuring memo uniqueness per challenge to prevent cross-challenge hash reuse.
+        if request.methodDetails.memo is None:
+            self._assert_challenge_bound_memo(
+                matched_logs,
+                challenge_id=challenge_id,
+                realm=realm,
+            )
+
+        if self._store is not None:
+            store_key = f"mpp:charge:{payload.hash.lower()}"
+            if not await self._store.put_if_absent(store_key, payload.hash):
+                raise VerificationError("Transaction hash already used")
+
         return Receipt.success(payload.hash)
 
+    def _assert_challenge_bound_memo(
+        self,
+        matched_logs: list[MatchedTransferLog],
+        challenge_id: str,
+        realm: str,
+    ) -> None:
+        from mpp.methods.tempo._attribution import verify_challenge_binding, verify_server
+
+        bound = any(
+            matched_log.kind == "memo"
+            and matched_log.memo is not None
+            and verify_server(matched_log.memo, realm)
+            and verify_challenge_binding(matched_log.memo, challenge_id)
+            for matched_log in matched_logs
+        )
+        if not bound:
+            raise VerificationError(
+                "Payment verification failed: memo is not bound to this challenge."
+            )
+
     def _verify_transfer_logs(
         self,
         receipt: dict[str, Any],
         request: ChargeRequest,
         expected_sender: str | None = None,
-    ) -> bool:
+    ) -> list[MatchedTransferLog]:
         """Check if receipt contains matching Transfer or TransferWithMemo logs.
 
         Args:
@@ -292,10 +338,13 @@ def _verify_transfer_logs(
                 Transfer log matches this address (for payer identity verification).
 
         Returns:
-            True if a matching Transfer/TransferWithMemo log is found,
-            False otherwise.
+            Matched logs in priority order, with memo logs before plain
+            transfers so downstream verification can inspect the memo that
+            actually satisfied the payment.
         """
         expected_memo = request.methodDetails.memo
+        memo_matches: list[MatchedTransferLog] = []
+        transfer_matches: list[MatchedTransferLog] = []
 
         for log in receipt.get("logs", []):
             if log.get("address", "").lower() != request.currency.lower():
@@ -315,9 +364,7 @@ def _verify_transfer_logs(
             if expected_sender and from_address.lower() != expected_sender.lower():
                 continue
 
-            if expected_memo:
-                if event_topic != TRANSFER_WITH_MEMO_TOPIC:
-                    continue
+            if event_topic == TRANSFER_WITH_MEMO_TOPIC:
                 # TransferWithMemo has 3 indexed params (from, to, memo)
                 # so memo is in topics[3] and only amount is in data
                 if len(topics) < 4:
@@ -326,22 +373,26 @@ def _verify_transfer_logs(
                 if len(data) < 66:
                     continue
                 amount = int(data[2:66], 16)
-                memo = topics[3]
-                memo_clean = expected_memo.lower()
-                if not memo_clean.startswith("0x"):
-                    memo_clean = "0x" + memo_clean
-                if amount == int(request.amount) and memo.lower() == memo_clean:
-                    return True
-            else:
-                if event_topic != TRANSFER_TOPIC:
+                if amount != int(request.amount):
                     continue
+                memo = topics[3]
+                if expected_memo:
+                    memo_clean = expected_memo.lower()
+                    if not memo_clean.startswith("0x"):
+                        memo_clean = "0x" + memo_clean
+                    if memo.lower() != memo_clean:
+                        continue
+                memo_matches.append(MatchedTransferLog(kind="memo", memo=memo))
+                continue
+
+            if event_topic == TRANSFER_TOPIC and expected_memo is None:
                 data = log.get("data", "0x")
                 if len(data) >= 66:
                     amount = int(data, 16)
                     if amount == int(request.amount):
-                        return True
+                        transfer_matches.append(MatchedTransferLog(kind="transfer"))
 
-        return False
+        return memo_matches + transfer_matches
 
     async def _verify_transaction(
         self,
@@ -449,6 +500,11 @@ async def _verify_transaction(
                 "Transaction must contain a Transfer log matching request parameters"
             )
 
+        if self._store is not None:
+            store_key = f"mpp:charge:{tx_hash.lower()}"
+            if not await self._store.put_if_absent(store_key, tx_hash):
+                raise VerificationError("Transaction hash already used")
+
         return Receipt.success(tx_hash)
 
     def _cosign_as_fee_payer(

src/mpp/methods/tempo/schemas.py

@@ -2,9 +2,9 @@
 
 from __future__ import annotations
 
-from typing import Annotated, Literal
+from typing import Annotated, Any, Literal
 
-from pydantic import BaseModel, Field
+from pydantic import BaseModel, Field, field_validator
 
 
 class MethodDetails(BaseModel):
@@ -15,6 +15,11 @@ class MethodDetails(BaseModel):
     feePayerUrl: str | None = None
     memo: str | None = None
 
+    @field_validator("memo", mode="before")
+    @classmethod
+    def normalize_empty_memo(cls, value: Any) -> Any:
+        return None if value == "" else value
+
 
 class ChargeRequest(BaseModel):
     """Request schema for the charge intent.