Skip to content

fix: RPC should return error on failed contract deployment instead of address #1276

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kstroobants wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: RPC should return error on failed contract deployment instead of address #1276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion backend/database_handler/transactions_processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ def _process_result(self, transaction_data: dict) -> dict:
return transaction_data

def get_transaction_by_hash(
self, transaction_hash: str, sim_config: dict | None = None
self, transaction_hash: str, sim_config: dict | None = None, hide_fields: bool = False
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix Black formatting to pass CI.

The pipeline failure indicates Black reformatted this line. The parameter list on a single line likely exceeds the line length limit.

🔧 Suggested fix 
     def get_transaction_by_hash(
-        self, transaction_hash: str, sim_config: dict | None = None, hide_fields: bool = False
+        self,
+        transaction_hash: str,
+        sim_config: dict | None = None,
+        hide_fields: bool = False,
     ) -> dict | 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.

Suggested change
self, transaction_hash: str, sim_config: dict | None = None, hide_fields: bool = False
self,
transaction_hash: str,
sim_config: dict | None = None,
hide_fields: bool = False,
🤖 Prompt for AI Agents 
In @backend/database_handler/transactions_processor.py at line 590, The method
signature line with parameters (self, transaction_hash: str, sim_config: dict |
None = None, hide_fields: bool = False) exceeds Black's line length and must be
reformatted; edit the method signature (the function/method definition
containing transaction_hash, sim_config and hide_fields) so the parameters are
broken across multiple lines (one parameter per line or wrapped to satisfy
Black), include a trailing comma if needed and preserve the existing type hints
and default values so Black will accept the file.

) -> dict | None:
transaction = (
self.session.query(Transactions)
Expand Down Expand Up @@ -630,6 +630,23 @@ def get_transaction_by_hash(
transaction_data = self._process_messages(transaction_data)
transaction_data = self._process_queue(transaction_data)
transaction_data = self._process_round_data(transaction_data)

if hide_fields:
if transaction_data[
"type"
] == TransactionType.DEPLOY_CONTRACT.value and not (
transaction_data["last_round"]["result"] == 6
and "leader_receipt" in transaction_data["consensus_data"]
and transaction_data["consensus_data"]["leader_receipt"] is not None
and len(transaction_data["consensus_data"]["leader_receipt"]) > 0
and transaction_data["consensus_data"]["leader_receipt"][0][
"execution_result"
]
== ExecutionResultStatus.SUCCESS.value
):
transaction_data["data"]["contract_address"] = None
if "contract_snapshot" in transaction_data:
del transaction_data["contract_snapshot"]
Comment on lines +634 to +649
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Potential TypeError if consensus_data is None; also use a named constant for magic number 6.

  1. If transaction_data["consensus_data"] is None and last_round["result"] == 6, the expression "leader_receipt" in transaction_data["consensus_data"] will raise TypeError: argument of type 'NoneType' is not iterable.

  2. The magic number 6 should be replaced with a named constant (e.g., ConsensusResult.MAJORITY_AGREE) for clarity and maintainability.

🔧 Suggested fix 
+        from backend.consensus.types import ConsensusResult
+
         if hide_fields:
             if transaction_data[
                 "type"
             ] == TransactionType.DEPLOY_CONTRACT.value and not (
-                transaction_data["last_round"]["result"] == 6
-                and "leader_receipt" in transaction_data["consensus_data"]
+                transaction_data["last_round"]["result"]
+                == int(ConsensusResult.MAJORITY_AGREE)
+                and transaction_data["consensus_data"] is not None
+                and "leader_receipt" in transaction_data["consensus_data"]
                 and transaction_data["consensus_data"]["leader_receipt"] is not None
                 and len(transaction_data["consensus_data"]["leader_receipt"]) > 0
                 and transaction_data["consensus_data"]["leader_receipt"][0][
                     "execution_result"
                 ]
                 == ExecutionResultStatus.SUCCESS.value
             ):
                 transaction_data["data"]["contract_address"] = None
                 if "contract_snapshot" in transaction_data:
                     del transaction_data["contract_snapshot"]
🤖 Prompt for AI Agents 
In @backend/database_handler/transactions_processor.py around lines 634 - 649,
Guard against None consensus_data and replace the magic number: in the block
inside transactions_processor where you check transaction_data["type"] ==
TransactionType.DEPLOY_CONTRACT.value and inspect
transaction_data["last_round"]["result"] == 6 and "leader_receipt" in
transaction_data["consensus_data"], first ensure consensus =
transaction_data.get("consensus_data") is not None and is a mapping (e.g., if
consensus and "leader_receipt" in consensus) before using the membership test to
avoid TypeError; also replace the literal 6 with the named constant (e.g.,
ConsensusResult.MAJORITY_AGREE.value) when comparing
transaction_data["last_round"]["result"] to improve clarity and maintainability,
keeping the rest of the existing checks on leader_receipt and execution_result
intact.

return transaction_data

def get_studio_transaction_by_hash(
Expand Down
25 changes: 17 additions & 8 deletions backend/protocol_rpc/endpoints.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,7 @@ def get_transaction_by_hash(
sim_config: dict | None = None,
) -> dict:
transaction = transactions_processor.get_transaction_by_hash(
transaction_hash, sim_config
transaction_hash, sim_config, True
)

if transaction is None:
Expand Down Expand Up @@ -1427,7 +1427,7 @@ def get_transaction_receipt(
event_signature = "NewTransaction(bytes32,address,address)"
event_signature_hash = eth_utils.keccak(text=event_signature).hex()

to_addr = transaction.get("to_address")
to_addr = transaction.get("to_address") if transaction.get("to_address") else None
from_addr = transaction.get("from_address")

logs = [
Expand Down Expand Up @@ -1457,6 +1457,19 @@ def get_transaction_receipt(
}
]

if (
transaction["type"] == TransactionType.DEPLOY_CONTRACT.value
and transaction["last_round"]["result"] == 6
and "leader_receipt" in transaction["consensus_data"]
and transaction["consensus_data"]["leader_receipt"] is not None
and len(transaction["consensus_data"]["leader_receipt"]) > 0
and transaction["consensus_data"]["leader_receipt"][0]["execution_result"]
== ExecutionResultStatus.SUCCESS.value
):
contract_address = transaction["to_address"]
else:
contract_address = None
Comment on lines +1460 to +1471
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Duplicated logic and magic number 6; also potential TypeError if consensus_data is None.

  1. This condition duplicates the logic in transactions_processor.py (lines 634-646). Consider extracting to a shared helper function for maintainability.

  2. The magic number 6 should use a named constant (e.g., ConsensusResult.MAJORITY_AGREE).

  3. If transaction["consensus_data"] is None, the "leader_receipt" in transaction["consensus_data"] check will raise TypeError.

♻️ Suggested refactor 
+from backend.consensus.types import ConsensusResult
+
+def _is_successful_deployment(transaction: dict) -> bool:
+    """Check if a DEPLOY_CONTRACT transaction was successfully deployed."""
+    return (
+        transaction["type"] == TransactionType.DEPLOY_CONTRACT.value
+        and transaction["last_round"]["result"] == int(ConsensusResult.MAJORITY_AGREE)
+        and transaction.get("consensus_data") is not None
+        and "leader_receipt" in transaction["consensus_data"]
+        and transaction["consensus_data"]["leader_receipt"] is not None
+        and len(transaction["consensus_data"]["leader_receipt"]) > 0
+        and transaction["consensus_data"]["leader_receipt"][0].get("execution_result")
+        == ExecutionResultStatus.SUCCESS.value
+    )
+
 # In get_transaction_receipt:
-    if (
-        transaction["type"] == TransactionType.DEPLOY_CONTRACT.value
-        and transaction["last_round"]["result"] == 6
-        and "leader_receipt" in transaction["consensus_data"]
-        and transaction["consensus_data"]["leader_receipt"] is not None
-        and len(transaction["consensus_data"]["leader_receipt"]) > 0
-        and transaction["consensus_data"]["leader_receipt"][0]["execution_result"]
-        == ExecutionResultStatus.SUCCESS.value
-    ):
-        contract_address = transaction["to_address"]
-    else:
-        contract_address = None
+    contract_address = transaction["to_address"] if _is_successful_deployment(transaction) else None

Committable suggestion skipped: line range outside the PR's diff.


receipt = {
"transactionHash": transaction_hash,
"transactionIndex": hex(0),
Expand All @@ -1466,11 +1479,7 @@ def get_transaction_receipt(
"to": to_addr,
"cumulativeGasUsed": hex(transaction.get("gas_used", 8000000)),
"gasUsed": hex(transaction.get("gas_used", 8000000)),
"contractAddress": (
transaction.get("contract_address")
if transaction.get("contract_address")
else None
),
"contractAddress": contract_address,
"logs": logs,
"logsBloom": "0x" + "00" * 256,
"status": hex(1 if transaction.get("status", True) else 0),
Expand All @@ -1485,7 +1494,7 @@ def get_block_by_hash(
full_tx: bool = False,
) -> dict | None:

transaction = transactions_processor.get_transaction_by_hash(block_hash)
transaction = transactions_processor.get_transaction_by_hash(block_hash, True)
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Type error: True is passed as sim_config instead of hide_fields.

The method signature is get_transaction_by_hash(transaction_hash, sim_config, hide_fields). Passing True positionally makes it the sim_config argument (which expects dict | None), not hide_fields.

🐛 Suggested fix 
-    transaction = transactions_processor.get_transaction_by_hash(block_hash, True)
+    transaction = transactions_processor.get_transaction_by_hash(block_hash, None, True)

Or use keyword argument for clarity:

-    transaction = transactions_processor.get_transaction_by_hash(block_hash, True)
+    transaction = transactions_processor.get_transaction_by_hash(block_hash, hide_fields=True)
📝 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.

Suggested change
transaction = transactions_processor.get_transaction_by_hash(block_hash, True)
transaction = transactions_processor.get_transaction_by_hash(block_hash, hide_fields=True)
🤖 Prompt for AI Agents 
In @backend/protocol_rpc/endpoints.py at line 1497, get_transaction_by_hash is
being called with True as a positional second argument, which is being bound to
sim_config (expects dict|None); change the call to pass sim_config explicitly
(e.g., None) and pass hide_fields as a keyword (hide_fields=True) or use
hide_fields=True and sim_config=None by name so the boolean goes to hide_fields;
update the call at transactions_processor.get_transaction_by_hash(block_hash,
...) accordingly.


if not transaction:
return None
Expand Down
Loading