feat: add sim_cancelTransaction RPC endpoint with auth and UI#1444
feat: add sim_cancelTransaction RPC endpoint with auth and UI#1444cristiam86 merged 4 commits intomainfrom
Conversation
- Backend: add cancel_transaction() handler in endpoints.py with
signature/admin_key auth (same pattern as sim_upgradeContractCode),
atomic SQL UPDATE guarded by status IN ('PENDING','ACTIVATED') AND
blocked_at IS NULL to avoid worker race conditions, and WebSocket
notification via msg_handler.send_transaction_status_update()
- Backend: register sim_cancelTransaction in rpc_methods.py
- Frontend: add cancelTransaction() to IJsonRpcService, JsonRpcService,
and transactions store (with ECDSA signing via viem for local/MetaMask)
- Frontend: add CANCEL button in TransactionItem.vue for PENDING/ACTIVATED
transactions; suppress spinner for CANCELED status
Closes DXP-736
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a new sim_cancelTransaction RPC and backend endpoint cancel_transaction to atomically cancel pending/activated transactions (with hosted/self-hosted access control via admin key or sender signature), frontend RPC/store/UI signing and cancel action, DB helper to perform conditional UPDATE, and tests. Duplicate endpoint insertion observed in the backend file. Changes
Sequence DiagramsequenceDiagram
participant UI as UI (Frontend)
participant Store as Store (Frontend)
participant Service as RPC Service
participant Backend as Backend Endpoint
participant DB as Database
participant WS as WebSocket
UI->>Store: cancelTransaction(txHash)
Store->>Store: Build message hash keccak256("cancel_transaction"+txHash)
Store->>Store: Sign message (local key or MetaMask)
Store->>Service: cancelTransaction(txHash, signature)
Service->>Backend: RPC sim_cancelTransaction(txHash, signature)
Backend->>Backend: Validate transaction_hash format
Backend->>DB: SELECT transaction by hash
alt not found
Backend-->>Service: Error (not found)
else found
alt hosted & admin_key provided
Backend->>DB: UPDATE set status=CANCELED where status IN (PENDING, ACTIVATED) AND blocked_at IS NULL
else signature provided
Backend->>Backend: Recover signer from signature
alt signer == sender
Backend->>DB: UPDATE set status=CANCELED where status IN (PENDING, ACTIVATED) AND blocked_at IS NULL
else
Backend-->>Service: Error (unauthorized)
end
else
Backend-->>Service: Error (unauthorized)
end
end
DB->>Backend: Rows updated / none
alt rows updated
Backend->>DB: COMMIT
Backend->>WS: Notify status CANCELED (transaction_hash)
Backend-->>Service: Return {transaction_hash, status:"CANCELED"}
else no rows updated
Backend-->>Service: Error (already processed / not cancellable)
end
Service-->>Store: Result / Error
Store-->>UI: Update UI state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/protocol_rpc/endpoints.py`:
- Around line 791-814: The current raw SQL UPDATE should be replaced with a call
to the canonical status updater: use
TransactionsProcessor.update_transaction_status(transaction,
TransactionStatus.CANCELED, update_current_status_changes=False) (or True if you
intentionally want bookkeeping) instead of executing raw SQL; call it with the
transaction object resolved from transaction_hash, commit the session after the
updater, and if the updater indicates no change (e.g., returns False or does not
transition because status/blocked_at prevented it) raise the same JSONRPCError
preserving transaction_hash and transaction.status.value; also ensure behavior
matches sim_updateTransactionStatus (the standard path) or document why you must
diverge.
In `@frontend/src/stores/transactions.ts`:
- Around line 130-135: The MetaMask branch is double-encoding the hash by
calling toHex(messageHash) before personal_sign; instead pass the raw hex hash
the same way as the local signer. Replace the personal_sign params so
personal_sign receives messageHash directly (mirror the local path that uses {
raw: messageHash }) — update the window.ethereum.request call (method
'personal_sign') to send messageHash (not toHex(messageHash)) with
account.address so MetaMask signs the actual hash bytes and backend verification
succeeds.
🧹 Nitpick comments (5)
backend/protocol_rpc/rpc_methods.py (1)
248-262: Use explicit| Nonefor optional parameters.Ruff (RUF013) flags
signature: str = Noneandadmin_key: str = Noneas implicitOptional, which PEP 484 prohibits. The existingupgrade_contract_code(lines 235-236) has the same issue, so this is a pre-existing pattern—but worth fixing in new code.Proposed fix
`@rpc.method`("sim_cancelTransaction") def cancel_transaction( transaction_hash: str, - signature: str = None, - admin_key: str = None, + signature: str | None = None, + admin_key: str | None = None, session: Session = Depends(get_db_session), msg_handler=Depends(get_message_handler), ) -> dict:backend/protocol_rpc/endpoints.py (3)
712-717: Redundant local imports.
TransactionStatusis already imported at module level (line 17), andosis already imported at module level (line 53). These local re-imports are unnecessary. The existingadmin_upgrade_contract_codehas the same pattern, but new code should avoid it.Proposed fix
from backend.database_handler.models import Transactions, TransactionStatus from eth_account.messages import encode_defunct from eth_account import Account from web3 import Web3 from sqlalchemy import text - import os
688-694: Add type hint formsg_handlerparameter.Other endpoint functions in this file use
IMessageHandleras the type for message handler parameters (e.g.,get_contract_schemaat line 829). As per coding guidelines, type hints should be included in all Python code.Proposed fix
def cancel_transaction( session: Session, transaction_hash: str, - msg_handler, + msg_handler: IMessageHandler, signature: str | None = None, admin_key: str | None = None, ) -> dict:As per coding guidelines: "Include type hints in all Python code".
761-783: Signature message lacks chain-id / domain separator — acceptable for studio but worth noting.The cancel message is
keccak256("cancel_transaction" + tx_hash_bytes). Since the tx_hash is unique per transaction, replay across transactions is prevented. However, there's no chain-id or domain separator, so the same signature could theoretically be replayed on a different GenLayer Studio instance sharing the same DB schema. This is likely acceptable for a studio/dev tool but worth documenting if the pattern is extended to production.frontend/src/stores/transactions.ts (1)
107-139: No user feedback when account is missing.If
accountisnull(line 116),signaturestaysundefinedand the RPC call proceeds without auth. In local dev mode this works, but in hosted mode the backend will reject with a generic error. Consider an early return or a more descriptive error.
Backend: 25 tests covering hash validation, auth (local/hosted/admin key/signature), atomic cancel race conditions, and success behavior. Frontend: 4 JsonRpcService tests + 3 transactions store tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
frontend/test/unit/stores/transactions.test.ts (1)
206-227: Tests at lines 206–215 and 217–227 are functionally identical.Both tests assert exactly the same thing:
mockCancelTransactionwas called with(hash, undefined). The second test ("should send no signature when no account is selected") doesn't set up any different conditions from the first, since thevi.doMockfor accounts already defaultsselectedAccounttonull.Collapse them into one, or differentiate the second by setting up a scenario with a selected account that has no signing capability.
tests/unit/test_cancel_transaction_endpoint.py (2)
167-178: Prefix unusedmock_encodeparameters with_to signal intent.The
mock_encodeargument is injected by the@patchdecorator but never referenced in the test bodies (also flagged by Ruff ARG002 at lines 171, 195, 252). Renaming to_mock_encodeacross all three test methods makes the intent explicit.Example fix (apply to all three methods)
def test_hosted_mode_valid_signature_from_sender( - self, mock_web3_class, mock_encode, mock_recover + self, mock_web3_class, _mock_encode, mock_recover ):
409-421: Fragile param extraction — and test name overpromises.The method is named
test_session_execute_called_with_correct_sqlbut only asserts the bind parameter (hash), not the SQL statement text. The param extraction at line 420 also relies on the call convention (positional vs keyword) ofsession.execute, which can silently break if the implementation changes how it passes args.Consider either:
- Renaming the test to
test_session_execute_called_with_correct_params, or- Also asserting something about the SQL text (e.g., checking
"CANCELED"or"blocked_at"appears in the rendered statement).
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/protocol_rpc/endpoints.py`:
- Around line 687-811: The cancel_transaction endpoint lacks a type hint for
msg_handler and its transaction_hash validation allows non-hex chars; update the
signature to type msg_handler as the IMessageHandler interface (or the concrete
handler class used elsewhere) and tighten the hash check in cancel_transaction
to verify the hex payload (e.g., ensure transaction_hash[2:] is 64 hex chars and
all characters are in [0-9a-fA-F]) before proceeding; keep the rest of the logic
(references: cancel_transaction, Transactions lookup,
TransactionsProcessor.cancel_transaction_if_available, and
msg_handler.send_transaction_status_update) unchanged so callers and tests pick
up the stronger validation and the declared handler type.
In `@tests/unit/consensus/test_previous_canceled_transaction.py`:
- Around line 12-74: Add explicit type hints to the two new async test functions
test_exec_transaction_continues_after_previous_canceled and
test_exec_transaction_waits_when_previous_in_processing: annotate the
consensus_algorithm parameter with its fixture/type (e.g., ConsensusAlgorithm or
appropriate test fixture type) and add the async function return type -> None;
ensure any other parameters (like transaction, transactions_processor) created
inside the test have local type hints where helpful. Update the function
signatures for both tests to include these parameter and return type annotations
so they conform to the project's typing guidelines.
- Around line 5-9: Remove the direct import of consensus_algorithm from the
module and register the fixture via pytest_plugins (add pytest_plugins =
["tests.unit.consensus.test_helpers"]) so pytest discovers the fixture without
redefinition; then add explicit type hints by importing typing.Callable and
typing.Any and annotate the consensus_algorithm fixture parameter as
Callable[..., Any] and all test functions with a return type of -> None (also
remove the now-unnecessary noqa on the consensus_algorithm import).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/consensus/base.pybackend/database_handler/transactions_processor.pybackend/protocol_rpc/endpoints.pyfrontend/src/components/Simulator/TransactionItem.vuefrontend/src/stores/transactions.tsfrontend/test/unit/stores/transactions.test.tstests/unit/consensus/test_previous_canceled_transaction.py
| from backend.database_handler.models import TransactionStatus | ||
| from tests.unit.consensus.test_helpers import ( | ||
| consensus_algorithm, # noqa: F401 - imported so pytest can discover fixture | ||
| init_dummy_transaction, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n tests/unit/consensus/test_previous_canceled_transaction.py | head -80Repository: genlayerlabs/genlayer-studio
Length of output: 3182
🏁 Script executed:
find tests/common -type f -name "*.py" | head -20Repository: genlayerlabs/genlayer-studio
Length of output: 174
🏁 Script executed:
rg "def consensus_algorithm" tests/ -A 5Repository: genlayerlabs/genlayer-studio
Length of output: 521
🏁 Script executed:
rg "pytest_plugins" tests/unit/consensus/ -lRepository: genlayerlabs/genlayer-studio
Length of output: 54
🏁 Script executed:
python -m ruff check tests/unit/consensus/test_previous_canceled_transaction.py --extend-select=F811Repository: genlayerlabs/genlayer-studio
Length of output: 2589
🏁 Script executed:
cat -n tests/unit/consensus/test_helpers.py | head -50Repository: genlayerlabs/genlayer-studio
Length of output: 2129
🏁 Script executed:
python -m ruff check tests/unit/consensus/test_previous_canceled_transaction.py --extend-select=ANNRepository: genlayerlabs/genlayer-studio
Length of output: 5236
🏁 Script executed:
rg "async def test_" tests/unit/consensus/test_helpers.py -A 2Repository: genlayerlabs/genlayer-studio
Length of output: 54
Fix F811 redefinition and add missing type hints.
Remove the direct consensus_algorithm import and register it via pytest_plugins to avoid the redefinition error. Add explicit type hints for both the test function return types (-> None) and the consensus_algorithm parameter.
🔧 Suggested changes
+from typing import TYPE_CHECKING
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from backend.database_handler.models import TransactionStatus
from tests.unit.consensus.test_helpers import (
- consensus_algorithm, # noqa: F401 - imported so pytest can discover fixture
init_dummy_transaction,
)
+
+if TYPE_CHECKING:
+ from tests.unit.consensus.test_helpers import ConsensusAlgorithm
+
+pytest_plugins = ["tests.unit.consensus.test_helpers"]Then update test signatures:
-async def test_exec_transaction_continues_after_previous_canceled(consensus_algorithm):
+async def test_exec_transaction_continues_after_previous_canceled(consensus_algorithm: "ConsensusAlgorithm") -> None:-async def test_exec_transaction_waits_when_previous_in_processing(consensus_algorithm):
+async def test_exec_transaction_waits_when_previous_in_processing(consensus_algorithm: "ConsensusAlgorithm") -> None:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from backend.database_handler.models import TransactionStatus | |
| from tests.unit.consensus.test_helpers import ( | |
| consensus_algorithm, # noqa: F401 - imported so pytest can discover fixture | |
| init_dummy_transaction, | |
| ) | |
| from typing import TYPE_CHECKING | |
| from unittest.mock import AsyncMock, MagicMock, patch | |
| import pytest | |
| from backend.database_handler.models import TransactionStatus | |
| from tests.unit.consensus.test_helpers import ( | |
| init_dummy_transaction, | |
| ) | |
| if TYPE_CHECKING: | |
| from tests.unit.consensus.test_helpers import ConsensusAlgorithm | |
| pytest_plugins = ["tests.unit.consensus.test_helpers"] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/consensus/test_previous_canceled_transaction.py` around lines 5 -
9, Remove the direct import of consensus_algorithm from the module and register
the fixture via pytest_plugins (add pytest_plugins =
["tests.unit.consensus.test_helpers"]) so pytest discovers the fixture without
redefinition; then add explicit type hints by importing typing.Callable and
typing.Any and annotate the consensus_algorithm fixture parameter as
Callable[..., Any] and all test functions with a return type of -> None (also
remove the now-unnecessary noqa on the consensus_algorithm import).
| @pytest.mark.asyncio | ||
| async def test_exec_transaction_continues_after_previous_canceled(consensus_algorithm): | ||
| transaction = init_dummy_transaction() | ||
| transactions_processor = MagicMock() | ||
| transactions_processor.get_previous_transaction.return_value = { | ||
| "appealed": False, | ||
| "appeal_undetermined": False, | ||
| "appeal_leader_timeout": False, | ||
| "appeal_validators_timeout": False, | ||
| "status": TransactionStatus.CANCELED.value, | ||
| } | ||
|
|
||
| with patch( | ||
| "backend.consensus.base.PendingState.handle", | ||
| new_callable=AsyncMock, | ||
| return_value=None, | ||
| ) as handle_mock: | ||
| await consensus_algorithm.exec_transaction( | ||
| transaction=transaction, | ||
| transactions_processor=transactions_processor, | ||
| chain_snapshot=MagicMock(), | ||
| accounts_manager=MagicMock(), | ||
| contract_snapshot_factory=lambda _address: MagicMock(), | ||
| contract_processor=MagicMock(), | ||
| node_factory=MagicMock(), | ||
| validators_snapshot=MagicMock(), | ||
| ) | ||
|
|
||
| transactions_processor.get_previous_transaction.assert_called_once_with( | ||
| transaction.hash | ||
| ) | ||
| handle_mock.assert_awaited_once() | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_exec_transaction_waits_when_previous_in_processing(consensus_algorithm): | ||
| transaction = init_dummy_transaction() | ||
| transactions_processor = MagicMock() | ||
| transactions_processor.get_previous_transaction.return_value = { | ||
| "appealed": False, | ||
| "appeal_undetermined": False, | ||
| "appeal_leader_timeout": False, | ||
| "appeal_validators_timeout": False, | ||
| "status": TransactionStatus.PROPOSING.value, | ||
| } | ||
|
|
||
| with patch( | ||
| "backend.consensus.base.PendingState.handle", | ||
| new_callable=AsyncMock, | ||
| return_value=None, | ||
| ) as handle_mock: | ||
| await consensus_algorithm.exec_transaction( | ||
| transaction=transaction, | ||
| transactions_processor=transactions_processor, | ||
| chain_snapshot=MagicMock(), | ||
| accounts_manager=MagicMock(), | ||
| contract_snapshot_factory=lambda _address: MagicMock(), | ||
| contract_processor=MagicMock(), | ||
| node_factory=MagicMock(), | ||
| validators_snapshot=MagicMock(), | ||
| ) | ||
|
|
||
| handle_mock.assert_not_awaited() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n tests/unit/consensus/test_previous_canceled_transaction.pyRepository: genlayerlabs/genlayer-studio
Length of output: 3182
🏁 Script executed:
cat -n tests/unit/consensus/test_helpers.pyRepository: genlayerlabs/genlayer-studio
Length of output: 44734
🏁 Script executed:
cd tests/unit/consensus && python -c "from backend.consensus.base import ConsensusAlgorithm; print('Import successful')"Repository: genlayerlabs/genlayer-studio
Length of output: 192
🏁 Script executed:
find . -name "base.py" -path "*/consensus/*" -type f | head -5Repository: genlayerlabs/genlayer-studio
Length of output: 99
🏁 Script executed:
grep -n "^class ConsensusAlgorithm" backend/consensus/base.pyRepository: genlayerlabs/genlayer-studio
Length of output: 101
Add explicit type hints to the new async tests.
Both test functions are missing parameter and return type annotations. Per the coding guidelines, include type hints in all Python code.
🧩 Suggested change
+from backend.consensus.base import ConsensusAlgorithm
+
`@pytest.mark.asyncio`
-async def test_exec_transaction_continues_after_previous_canceled(consensus_algorithm):
+async def test_exec_transaction_continues_after_previous_canceled(
+ consensus_algorithm: ConsensusAlgorithm,
+) -> None:
@@
`@pytest.mark.asyncio`
-async def test_exec_transaction_waits_when_previous_in_processing(consensus_algorithm):
+async def test_exec_transaction_waits_when_previous_in_processing(
+ consensus_algorithm: ConsensusAlgorithm,
+) -> None:🧰 Tools
🪛 Ruff (0.15.2)
[error] 13-13: Redefinition of unused consensus_algorithm from line 7: consensus_algorithm redefined here
(F811)
[error] 47-47: Redefinition of unused consensus_algorithm from line 7: consensus_algorithm redefined here
(F811)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/consensus/test_previous_canceled_transaction.py` around lines 12 -
74, Add explicit type hints to the two new async test functions
test_exec_transaction_continues_after_previous_canceled and
test_exec_transaction_waits_when_previous_in_processing: annotate the
consensus_algorithm parameter with its fixture/type (e.g., ConsensusAlgorithm or
appropriate test fixture type) and add the async function return type -> None;
ensure any other parameters (like transaction, transactions_processor) created
inside the test have local type hints where helpful. Update the function
signatures for both tests to include these parameter and return type annotations
so they conform to the project's typing guidelines.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
backend/protocol_rpc/endpoints.py (2)
691-691:msg_handlerstill missing its type annotation.
IMessageHandleris already imported at the top of the file (line 24) and used as the annotated type on every other endpoint that takes a message handler. Leavingmsg_handleruntyped here is inconsistent.def cancel_transaction( session: Session, transaction_hash: str, - msg_handler, + msg_handler: IMessageHandler, signature: str | None = None, admin_key: str | None = None, ) -> dict:As per coding guidelines: "Include type hints in all Python code."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/protocol_rpc/endpoints.py` at line 691, The parameter msg_handler is missing its type annotation; update the function signature where msg_handler is declared to annotate it as IMessageHandler (import already present) so it matches other endpoints that accept a message handler—ensure any references to msg_handler within that function remain unchanged and the annotation uses the existing IMessageHandler symbol.
719-728: Hash validation still missing hex-character check.The existing
update_transaction_statusendpoint (line 1806) validates non-hex characters withint(transaction_hash, 16). This endpoint doesn't, so inputs like"0x" + "ZZ" * 32would pass validation and produce a confusing DB miss rather than a clear 400 error.🛡️ Proposed fix
if ( not transaction_hash or not transaction_hash.startswith("0x") or len(transaction_hash) != 66 ): raise JSONRPCError( code=-32602, message="Invalid transaction hash format", data={}, ) + try: + int(transaction_hash, 16) + except ValueError as e: + raise JSONRPCError( + code=-32602, + message="Invalid transaction hash format: contains non-hexadecimal characters", + data={}, + ) from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/protocol_rpc/endpoints.py` around lines 719 - 728, The current validation allows non-hex chars (e.g. "0xZZ...") because it only checks prefix/length; add a hex-character check by attempting to parse the hex payload with int(transaction_hash[2:], 16) inside a try/except and raise the same JSONRPCError on ValueError; specifically update the block that inspects transaction_hash (use the transaction_hash variable and JSONRPCError) to catch non-hex input and return the "Invalid transaction hash format" error instead of letting it fall through to a DB miss (mirror the approach used in update_transaction_status).
🧹 Nitpick comments (2)
backend/protocol_rpc/endpoints.py (2)
806-811: UseTransactionStatus.CANCELED.valueinstead of the hardcoded string.Hardcoding
"CANCELED"in both the WebSocket call and the return dict creates a silent divergence risk if the enum value is ever renamed.♻️ Proposed fix
+ canceled_status = TransactionStatus.CANCELED.value # Notify frontend via WebSocket - msg_handler.send_transaction_status_update(transaction_hash, "CANCELED") + msg_handler.send_transaction_status_update(transaction_hash, canceled_status) return { "transaction_hash": transaction_hash, - "status": "CANCELED", + "status": canceled_status, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/protocol_rpc/endpoints.py` around lines 806 - 811, Replace the hardcoded "CANCELED" string with the enum value to avoid divergence: call msg_handler.send_transaction_status_update(transaction_hash, TransactionStatus.CANCELED.value) and return {"transaction_hash": transaction_hash, "status": TransactionStatus.CANCELED.value}; update references in this endpoint where send_transaction_status_update and the response dict are set so both use TransactionStatus.CANCELED.value consistently.
712-716: Remove redundantimport osinside function body.
osis already imported at module level (line 53). The in-functionimport osis a no-op but misleads readers into thinkingosis unavailable in the outer scope.♻️ Proposed fix
from backend.database_handler.models import Transactions from eth_account.messages import encode_defunct from eth_account import Account from web3 import Web3 - import os🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/protocol_rpc/endpoints.py` around lines 712 - 716, Remove the redundant "import os" that appears inside the function body in backend/protocol_rpc/endpoints.py (the local import near the Transactions/encode_defunct/Account/Web3 imports); rely on the module-level os already imported at top of the file and delete the in-function import to avoid confusion and potential shadowing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/protocol_rpc/endpoints.py`:
- Around line 795-803: The error payload uses a stale transaction.status value
because status was read before calling cancel_transaction_if_available; after
detecting was_cancelled is False you should re-query the latest transaction
state and use that current status in the JSONRPCError data. Locate the cancel
flow around cancel_transaction_if_available and replace the pre-read
transaction.status value with a fresh fetch (using the same transaction lookup
used earlier keyed by transaction_hash) so the JSONRPCError includes the actual
terminal status in its data field (transaction_hash and the re-fetched
transaction.status.value).
- Around line 687-811: Add a type annotation for the msg_handler parameter
(e.g., msg_handler: MessageHandler) and import that MessageHandler type where
the module-level imports live; validate the transaction_hash hex payload by
wrapping int(transaction_hash[2:], 16) in a try/except and raise JSONRPCError on
ValueError to reject non-hex characters; remove the local "import os" and rely
on the module-level os import; replace hardcoded "CANCELED" string usages with
TransactionStatus.CANCELED.value (import TransactionStatus from
backend.database_handler.models or the appropriate enum) and when constructing
the error response on failed cancel, refresh or re-query the Transactions row
(or fetch its status after
TransactionsProcessor.cancel_transaction_if_available) before reading
transaction.status.value so the returned status is current.
---
Duplicate comments:
In `@backend/protocol_rpc/endpoints.py`:
- Line 691: The parameter msg_handler is missing its type annotation; update the
function signature where msg_handler is declared to annotate it as
IMessageHandler (import already present) so it matches other endpoints that
accept a message handler—ensure any references to msg_handler within that
function remain unchanged and the annotation uses the existing IMessageHandler
symbol.
- Around line 719-728: The current validation allows non-hex chars (e.g.
"0xZZ...") because it only checks prefix/length; add a hex-character check by
attempting to parse the hex payload with int(transaction_hash[2:], 16) inside a
try/except and raise the same JSONRPCError on ValueError; specifically update
the block that inspects transaction_hash (use the transaction_hash variable and
JSONRPCError) to catch non-hex input and return the "Invalid transaction hash
format" error instead of letting it fall through to a DB miss (mirror the
approach used in update_transaction_status).
---
Nitpick comments:
In `@backend/protocol_rpc/endpoints.py`:
- Around line 806-811: Replace the hardcoded "CANCELED" string with the enum
value to avoid divergence: call
msg_handler.send_transaction_status_update(transaction_hash,
TransactionStatus.CANCELED.value) and return {"transaction_hash":
transaction_hash, "status": TransactionStatus.CANCELED.value}; update references
in this endpoint where send_transaction_status_update and the response dict are
set so both use TransactionStatus.CANCELED.value consistently.
- Around line 712-716: Remove the redundant "import os" that appears inside the
function body in backend/protocol_rpc/endpoints.py (the local import near the
Transactions/encode_defunct/Account/Web3 imports); rely on the module-level os
already imported at top of the file and delete the in-function import to avoid
confusion and potential shadowing.
| ####### CANCEL TRANSACTION ENDPOINTS ####### | ||
| def cancel_transaction( | ||
| session: Session, | ||
| transaction_hash: str, | ||
| msg_handler, | ||
| signature: str | None = None, | ||
| admin_key: str | None = None, | ||
| ) -> dict: | ||
| """ | ||
| Cancel a pending or activated transaction. Returns immediately with status. | ||
|
|
||
| Access control: | ||
| - Local (no env vars): open access | ||
| - Hosted/Self-hosted: admin_key allows ANY transaction, signature allows own transactions | ||
|
|
||
| Args: | ||
| session: Database session | ||
| transaction_hash: Hash of the transaction to cancel | ||
| msg_handler: Message handler for WebSocket notifications | ||
| signature: Hex-encoded signature from tx sender (required in hosted mode unless admin_key) | ||
| admin_key: Admin API key for full access to any transaction | ||
|
|
||
| Returns: | ||
| dict with transaction_hash and status | ||
| """ | ||
| from backend.database_handler.models import Transactions | ||
| from eth_account.messages import encode_defunct | ||
| from eth_account import Account | ||
| from web3 import Web3 | ||
| import os | ||
|
|
||
| # Validate transaction hash format | ||
| if ( | ||
| not transaction_hash | ||
| or not transaction_hash.startswith("0x") | ||
| or len(transaction_hash) != 66 | ||
| ): | ||
| raise JSONRPCError( | ||
| code=-32602, | ||
| message="Invalid transaction hash format", | ||
| data={}, | ||
| ) | ||
|
|
||
| # Look up the transaction | ||
| transaction = ( | ||
| session.query(Transactions).filter_by(hash=transaction_hash).one_or_none() | ||
| ) | ||
| if not transaction: | ||
| raise NotFoundError( | ||
| message="Transaction not found", | ||
| data={"transaction_hash": transaction_hash}, | ||
| ) | ||
|
|
||
| is_hosted = os.getenv("VITE_IS_HOSTED") == "true" | ||
| admin_api_key = os.getenv("ADMIN_API_KEY") | ||
|
|
||
| # Check if authorization is needed (hosted or self-hosted with key configured) | ||
| needs_auth = is_hosted or admin_api_key | ||
|
|
||
| if needs_auth: | ||
| # Option 1: Admin key grants full access to ANY transaction | ||
| if admin_api_key and admin_key == admin_api_key: | ||
| pass # Authorized - proceed with cancel | ||
|
|
||
| # Option 2: Signature from tx sender grants access to own transactions | ||
| elif signature: | ||
| if not transaction.from_address: | ||
| raise JSONRPCError( | ||
| code=-32000, | ||
| message="Transaction has no sender - only admin key can cancel", | ||
| data={}, | ||
| ) | ||
|
|
||
| try: | ||
| # Message: keccak256("cancel_transaction" + tx_hash_bytes) | ||
| # tx_hash is unique, so no nonce needed for replay protection | ||
| message_hash = Web3.keccak( | ||
| b"cancel_transaction" + Web3.to_bytes(hexstr=transaction_hash) | ||
| ) | ||
| message = encode_defunct(primitive=message_hash) | ||
| signer = Account.recover_message(message, signature=signature) | ||
|
|
||
| if signer.lower() != transaction.from_address.lower(): | ||
| raise JSONRPCError( | ||
| code=-32000, | ||
| message="Only transaction sender can cancel", | ||
| data={"signer": signer, "sender": transaction.from_address}, | ||
| ) | ||
| except JSONRPCError: | ||
| raise | ||
| except Exception as e: | ||
| raise JSONRPCError( | ||
| code=-32000, | ||
| message=f"Invalid signature: {e!s}", | ||
| data={}, | ||
| ) from e | ||
| else: | ||
| raise JSONRPCError( | ||
| code=-32000, | ||
| message="Cancel requires admin key or sender signature", | ||
| data={}, | ||
| ) | ||
|
|
||
| # Atomic cancel - only succeeds if tx is still pending/activated and not claimed by worker | ||
| was_cancelled = TransactionsProcessor.cancel_transaction_if_available( | ||
| session, transaction_hash | ||
| ) | ||
|
|
||
| if not was_cancelled: | ||
| raise JSONRPCError( | ||
| code=-32000, | ||
| message="Transaction cannot be cancelled: already being processed or in a terminal state", | ||
| data={ | ||
| "transaction_hash": transaction_hash, | ||
| "status": transaction.status.value, | ||
| }, | ||
| ) | ||
|
|
||
| # Notify frontend via WebSocket | ||
| msg_handler.send_transaction_status_update(transaction_hash, "CANCELED") | ||
|
|
||
| return { | ||
| "transaction_hash": transaction_hash, | ||
| "status": "CANCELED", | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the number of `cancel_transaction` function definitions in the endpoints file
echo "=== Checking for duplicate cancel_transaction definitions ==="
rg -n "^def cancel_transaction" backend/protocol_rpc/endpoints.py
echo ""
echo "=== Line count and context around lines 687-811 ==="
wc -l backend/protocol_rpc/endpoints.pyRepository: genlayerlabs/genlayer-studio
Length of output: 253
🏁 Script executed:
#!/bin/bash
# Get the full cancel_transaction function and surrounding context
sed -n '685,815p' backend/protocol_rpc/endpoints.py | cat -nRepository: genlayerlabs/genlayer-studio
Length of output: 5495
🏁 Script executed:
#!/bin/bash
# Check module-level imports of os in the file
head -80 backend/protocol_rpc/endpoints.py | grep -n "^import os\|^from os"Repository: genlayerlabs/genlayer-studio
Length of output: 84
Add type hint to msg_handler parameter and validate hex characters in transaction hash.
Line 691: msg_handler is missing a type hint. This violates the coding guideline requiring type hints on all Python code. Provide the correct type annotation.
Lines 719–728: Transaction hash validation checks only the format (0x prefix, 66 characters), but does not validate that the remaining 64 characters are valid hexadecimal. Add validation using int(transaction_hash, 16) inside a try-except to reject malformed hashes early.
Additional minor improvements:
- Line 716:
import osis redundant;osis already imported at line 53. Remove the local import. - Lines 806, 810: Replace hardcoded
"CANCELED"strings withTransactionStatus.CANCELED.valuefor consistency and maintainability. - Line 801: The
transaction.status.valuein the error response is fetched before the cancel attempt and may be stale if the transaction state changes concurrently. Consider deferring or refreshing.
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 770-774: Abstract raise to an inner function
(TRY301)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/protocol_rpc/endpoints.py` around lines 687 - 811, Add a type
annotation for the msg_handler parameter (e.g., msg_handler: MessageHandler) and
import that MessageHandler type where the module-level imports live; validate
the transaction_hash hex payload by wrapping int(transaction_hash[2:], 16) in a
try/except and raise JSONRPCError on ValueError to reject non-hex characters;
remove the local "import os" and rely on the module-level os import; replace
hardcoded "CANCELED" string usages with TransactionStatus.CANCELED.value (import
TransactionStatus from backend.database_handler.models or the appropriate enum)
and when constructing the error response on failed cancel, refresh or re-query
the Transactions row (or fetch its status after
TransactionsProcessor.cancel_transaction_if_available) before reading
transaction.status.value so the returned status is current.
| if not was_cancelled: | ||
| raise JSONRPCError( | ||
| code=-32000, | ||
| message="Transaction cannot be cancelled: already being processed or in a terminal state", | ||
| data={ | ||
| "transaction_hash": transaction_hash, | ||
| "status": transaction.status.value, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Error data may show a stale status after a failed cancel.
transaction.status is read before cancel_transaction_if_available executes (line 732). If a consensus worker claimed and advanced the transaction in the interim, the error data will display the pre-race status (e.g., still PENDING) rather than the actual terminal state that caused the rejection, which can confuse callers.
🛠️ Proposed fix — re-fetch status on failure
if not was_cancelled:
+ # Re-query to get the current (post-race) status for an accurate error payload
+ session.refresh(transaction)
raise JSONRPCError(
code=-32000,
message="Transaction cannot be cancelled: already being processed or in a terminal state",
data={
"transaction_hash": transaction_hash,
"status": transaction.status.value,
},
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/protocol_rpc/endpoints.py` around lines 795 - 803, The error payload
uses a stale transaction.status value because status was read before calling
cancel_transaction_if_available; after detecting was_cancelled is False you
should re-query the latest transaction state and use that current status in the
JSONRPCError data. Locate the cancel flow around cancel_transaction_if_available
and replace the pre-read transaction.status value with a fresh fetch (using the
same transaction lookup used earlier keyed by transaction_hash) so the
JSONRPCError includes the actual terminal status in its data field
(transaction_hash and the re-fetched transaction.status.value).
|
🎉 This PR is included in version 0.103.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Closes DXP-736
What
sim_cancelTransactionJSON-RPC endpoint in the backend with the same auth pattern assim_upgradeContractCode(admin API key or ECDSA signature from the transaction sender)UPDATE ... WHERE status IN ('PENDING', 'ACTIVATED') AND blocked_at IS NULLto prevent race conditions with consensus workerscancelTransaction()to the API service and transactions store (signs with viem for local accounts or MetaMask)TransactionItem.vuefor PENDING/ACTIVATED transactions; suppressed the loading spinner for CANCELED statusWhy
Testing done
vue-tsc --noEmit) passes cleanDecisions made
keccak256("cancel_transaction" + tx_hash_bytes)— tx hashes are globally unique so no extra nonce is needed for replay protectionfrom_address = NULL(e.g., upgrade transactions) can only be cancelled via admin key, not signatureblocked_at IS NOT NULL, the worker has already claimed the transaction and the cancel is rejected with a clear errorChecks
Reviewing tips
backend/protocol_rpc/endpoints.py— the newcancel_transaction()function follows the exact same structure asadmin_upgrade_contract_code()above itstores/transactions.tsmirrorsuseContractQueries.tsupgradeContract()User facing release notes
Summary by CodeRabbit
New Features
UI
API
Backend
Tests