Skip to content

fix(nano): improve error handling #1321

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

Closed
glevco wants to merge 2 commits into from
Closed

fix(nano): improve error handling #1321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f27c34
fix(nano): improve error handling
glevco Jul 16, 2025
3a08627
review changes
glevco Jul 28, 2025
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
46 changes: 46 additions & 0 deletions extras/custom_checks.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,48 @@ function check_do_not_import_twisted_reactor_directly() {
return 0
}

function check_inherit_from_nc_user_exception() {
PATTERN='class .*\((NCUserException|NCFail)\)'

if grep -RIE "$PATTERN" "hathor"; then
echo 'do not inherit from NCUserException or NCFail, this is only meant to be inherited by blueprint exceptions, created by users.'
echo 'we may allow blueprints to catch these exceptions at some point.'
return 1
fi
return 0
}

function check_raise_nc_user_exception() {
PATTERN='raise (NCUserException|NCFail)'

if grep -RIE "$PATTERN" "hathor" | grep -v '# skip-raise-nc-user-exception'; then
echo 'do not raise NCUserException or NCFail, this is only meant to be raised by blueprints, created by users.'
echo 'we may allow blueprints to catch these exceptions at some point.'
return 1
fi
return 0
}

function check_inherit_from_nc_transaction_fail() {
PATTERN='class .*\(__NCTransactionFail__\)'

if grep -RIE "$PATTERN" "${SOURCE_DIRS[@]}" | grep -v '# skip-inherit-from-nc-tx-fail'; then
echo 'do not inherit from __NCTransactionFail__, this is only meant to be inherited in the error_handling module.'
return 1
fi
return 0
}

function check_raise_nc_unhandled_exception() {
PATTERN='raise (__NCUnhandledInternalException__|__NCUnhandledUserException__)'

if grep -RIE "$PATTERN" "${SOURCE_DIRS[@]}" | grep -v '# skip-raise-nc-unhandled-exception'; then
echo 'do not raise __NCUnhandledInternalException__ or __NCUnhandledUserException__, this is only meant to be raised by the error_handling module.'
return 1
fi
return 0
}

# List of functions to be executed
checks=(
check_version_match
Expand All @@ -123,6 +165,10 @@ checks=(
check_do_not_import_tests_in_hathor
check_do_not_import_from_hathor_in_entrypoints
check_do_not_import_twisted_reactor_directly
check_inherit_from_nc_user_exception
check_raise_nc_user_exception
check_inherit_from_nc_transaction_fail
check_raise_nc_unhandled_exception
)

# Initialize a variable to track if any check fails
Expand Down
2 changes: 1 addition & 1 deletion hathor/cli/events_simulator/scenario.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ def initialize(self, ctx: Context) -> None:
def fail(self, ctx: Context) -> None:
# This will not be emitted because the tx will fail.
self.syscall.emit_event(b'test event on fail')
raise NCFail
raise NCFail # skip-raise-nc-user-exception

@public
def call_another(self, ctx: Context, contract_id: ContractId) -> None:
Expand Down
12 changes: 9 additions & 3 deletions hathor/consensus/block_consensus.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ def execute_nano_contracts(self, block: Block) -> None:
def _nc_execute_calls(self, block: Block, *, is_reorg: bool) -> None:
"""Internal method to execute the method calls for transactions confirmed by this block.
"""
from hathor.nanocontracts import NC_EXECUTION_FAIL_ID, NCFail
from hathor.nanocontracts import NC_EXECUTION_FAIL_ID
from hathor.nanocontracts.error_handling import __NCTransactionFail__
from hathor.nanocontracts.types import Address

assert self._settings.ENABLE_NANO_CONTRACTS
Expand Down Expand Up @@ -194,10 +195,15 @@ def _nc_execute_calls(self, block: Block, *, is_reorg: bool) -> None:
continue

runner = self._runner_factory.create(block_storage=block_storage, seed=seed_hasher.digest())
exception_and_tb: tuple[NCFail, str] | None = None
exception_and_tb: tuple[__NCTransactionFail__, str] | None = None

try:
runner.execute_from_tx(tx)
except NCFail as e:
except __NCTransactionFail__ as e:
# These are the exception types that will make an NC transaction fail.
# Any other exception will bubble up and crash the full node.
Copy link
Member

@msbrogli msbrogli Jul 22, 2025

Choose a reason for hiding this comment

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

Even if there's a bug that generates an exception, is crashing the full node the best solution? I guess we could safely log that an unhandled exception happened in our code and void the transaction. In this case, the fix would need to be activated using the feature activation service.

Copy link
Contributor Author

@glevco glevco Jul 28, 2025

Choose a reason for hiding this comment

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

Wouldn't that defeat the purpose of this PR? What it tries to do is make sure NCs fail only when the exception is not a bug from our code. Even then, it doesn't fully accomplish this task because of the Risks and Discussion section in the PR description.

If we accept the fact that we'll fail NCs with exceptions from our own bugs, isn't that the same as doing the except Exception on the execution entrypoint?

Also, it's worth mentioning that failing NCs with our bugs is more than just an inconvenience for the users: it could make blueprints reach unrecoverable states that would cause locked funds in contracts, until fixed with feature activation.

# See the `error_handling` module for more information.

kwargs: dict[str, Any] = {}
if tx.name:
kwargs['__name'] = tx.name
Expand Down
7 changes: 1 addition & 6 deletions hathor/manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
from hathor.feature_activation.bit_signaling_service import BitSignalingService
from hathor.mining import BlockTemplate, BlockTemplates
from hathor.mining.cpu_mining_service import CpuMiningService
from hathor.nanocontracts.exception import NanoContractDoesNotExist
from hathor.nanocontracts.runner import Runner
from hathor.nanocontracts.runner.runner import RunnerFactory
from hathor.nanocontracts.storage import NCBlockStorage, NCContractStorage
Expand Down Expand Up @@ -406,11 +405,7 @@ def get_nc_storage(self, block: Block, nc_id: VertexId) -> NCContractStorage:
"""Return a contract storage with the contract state at a given block."""
from hathor.nanocontracts.types import ContractId, VertexId as NCVertexId
block_storage = self.get_nc_block_storage(block)
try:
contract_storage = block_storage.get_contract_storage(ContractId(NCVertexId(nc_id)))
except KeyError:
raise NanoContractDoesNotExist(nc_id.hex())
return contract_storage
return block_storage.get_contract_storage(ContractId(NCVertexId(nc_id)))

def get_best_block_nc_storage(self, nc_id: VertexId) -> NCContractStorage:
"""Return a contract storage with the contract state at the best block."""
Expand Down
7 changes: 5 additions & 2 deletions hathor/nanocontracts/blueprint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from typing import TYPE_CHECKING, Any, final

from hathor.nanocontracts.blueprint_env import BlueprintEnvironment
from hathor.nanocontracts.error_handling import internal_code_called_from_user_code
from hathor.nanocontracts.exception import BlueprintSyntaxError
from hathor.nanocontracts.nc_types.utils import pretty_type
from hathor.nanocontracts.types import NC_FALLBACK_METHOD, NC_INITIALIZE_METHOD, NC_METHOD_TYPE_ATTR, NCMethodType
Expand Down Expand Up @@ -131,14 +132,16 @@ class MyBlueprint(Blueprint):
def __init__(self, env: BlueprintEnvironment) -> None:
self.__env = env

@final
@property
@internal_code_called_from_user_code
@final
def syscall(self) -> BlueprintEnvironment:
"""Return the syscall provider for the current contract."""
return self.__env

@final
@property
@internal_code_called_from_user_code
@final
def log(self) -> NCLogger:
"""Return the logger for the current contract."""
return self.syscall.__log__
23 changes: 22 additions & 1 deletion hathor/nanocontracts/blueprint_env.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

from typing import TYPE_CHECKING, Any, Optional, Sequence, final

from hathor.nanocontracts.error_handling import internal_code_called_from_user_code
from hathor.nanocontracts.storage import NCContractStorage
from hathor.nanocontracts.types import Amount, BlueprintId, ContractId, NCAction, TokenUid

Expand Down Expand Up @@ -45,24 +46,28 @@ def __init__(
# XXX: we could replace dict|None with a Cache that can be disabled, cleared, limited, etc
self.__cache__: dict[str, Any] | None = None if disable_cache else {}

@final
@property
@internal_code_called_from_user_code
@final
def rng(self) -> NanoRNG:
"""Return an RNG for the current contract."""
return self.__runner.syscall_get_rng()

@internal_code_called_from_user_code
@final
def get_contract_id(self) -> ContractId:
"""Return the current contract id."""
return self.__runner.get_current_contract_id()

@internal_code_called_from_user_code
@final
def get_blueprint_id(self, contract_id: Optional[ContractId] = None) -> BlueprintId:
"""Return the blueprint id of a nano contract. By default, it returns for the current contract."""
if contract_id is None:
contract_id = self.get_contract_id()
return self.__runner.get_blueprint_id(contract_id)

@internal_code_called_from_user_code
def get_balance_before_current_call(
self,
token_uid: Optional[TokenUid] = None,
Expand All @@ -78,6 +83,7 @@ def get_balance_before_current_call(
balance = self.__runner.get_balance_before_current_call(contract_id, token_uid)
return Amount(balance.value)

@internal_code_called_from_user_code
def get_current_balance(
self,
token_uid: Optional[TokenUid] = None,
Expand All @@ -93,6 +99,7 @@ def get_current_balance(
balance = self.__runner.get_current_balance(contract_id, token_uid)
return Amount(balance.value)

@internal_code_called_from_user_code
@final
def can_mint_before_current_call(
self,
Expand All @@ -110,6 +117,7 @@ def can_mint_before_current_call(
balance = self.__runner.get_balance_before_current_call(contract_id, token_uid)
return balance.can_mint

@internal_code_called_from_user_code
@final
def can_mint(
self,
Expand All @@ -127,6 +135,7 @@ def can_mint(
balance = self.__runner.get_current_balance(contract_id, token_uid)
return balance.can_mint

@internal_code_called_from_user_code
@final
def can_melt_before_current_call(
self,
Expand All @@ -144,6 +153,7 @@ def can_melt_before_current_call(
balance = self.__runner.get_balance_before_current_call(contract_id, token_uid)
return balance.can_melt

@internal_code_called_from_user_code
@final
def can_melt(
self,
Expand All @@ -161,6 +171,7 @@ def can_melt(
balance = self.__runner.get_current_balance(contract_id, token_uid)
return balance.can_melt

@internal_code_called_from_user_code
@final
def call_public_method(
self,
Expand All @@ -173,6 +184,7 @@ def call_public_method(
"""Call a public method of another contract."""
return self.__runner.syscall_call_another_contract_public_method(nc_id, method_name, actions, args, kwargs)

@internal_code_called_from_user_code
@final
def proxy_call_public_method(
self,
Expand All @@ -185,6 +197,7 @@ def proxy_call_public_method(
"""Execute a proxy call to a public method of another blueprint."""
return self.__runner.syscall_proxy_call_public_method(blueprint_id, method_name, actions, args, kwargs)

@internal_code_called_from_user_code
@final
def proxy_call_public_method_nc_args(
self,
Expand All @@ -196,26 +209,31 @@ def proxy_call_public_method_nc_args(
"""Execute a proxy call to a public method of another blueprint."""
return self.__runner.syscall_proxy_call_public_method_nc_args(blueprint_id, method_name, actions, nc_args)

@internal_code_called_from_user_code
@final
def call_view_method(self, nc_id: ContractId, method_name: str, *args: Any, **kwargs: Any) -> Any:
"""Call a view method of another contract."""
return self.__runner.syscall_call_another_contract_view_method(nc_id, method_name, args, kwargs)

@internal_code_called_from_user_code
@final
def revoke_authorities(self, token_uid: TokenUid, *, revoke_mint: bool, revoke_melt: bool) -> None:
"""Revoke authorities from this nano contract."""
self.__runner.syscall_revoke_authorities(token_uid=token_uid, revoke_mint=revoke_mint, revoke_melt=revoke_melt)

@internal_code_called_from_user_code
@final
def mint_tokens(self, token_uid: TokenUid, amount: int) -> None:
"""Mint tokens and add them to the balance of this nano contract."""
self.__runner.syscall_mint_tokens(token_uid=token_uid, amount=amount)

@internal_code_called_from_user_code
@final
def melt_tokens(self, token_uid: TokenUid, amount: int) -> None:
"""Melt tokens by removing them from the balance of this nano contract."""
self.__runner.syscall_melt_tokens(token_uid=token_uid, amount=amount)

@internal_code_called_from_user_code
@final
def create_contract(
self,
Expand All @@ -228,11 +246,13 @@ def create_contract(
"""Create a new contract."""
return self.__runner.syscall_create_another_contract(blueprint_id, salt, actions, args, kwargs)

@internal_code_called_from_user_code
@final
def emit_event(self, data: bytes) -> None:
"""Emit a custom event from a Nano Contract."""
self.__runner.syscall_emit_event(data)

@internal_code_called_from_user_code
@final
def create_token(
self,
Expand All @@ -251,6 +271,7 @@ def create_token(
melt_authority,
)

@internal_code_called_from_user_code
@final
def change_blueprint(self, blueprint_id: BlueprintId) -> None:
"""Change the blueprint of this contract."""
Expand Down
5 changes: 3 additions & 2 deletions hathor/nanocontracts/context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
from typing import TYPE_CHECKING, Any, Sequence, final

from hathor.crypto.util import get_address_b58_from_bytes
from hathor.nanocontracts.exception import NCFail, NCInvalidContext
from hathor.nanocontracts.error_handling import NCInternalException
from hathor.nanocontracts.exception import NCInvalidContext
from hathor.nanocontracts.types import Address, ContractId, NCAction, TokenUid
from hathor.nanocontracts.vertex_data import VertexData
from hathor.transaction.exceptions import TxValidationError
Expand Down Expand Up @@ -108,7 +109,7 @@ def get_single_action(self, token_uid: TokenUid) -> NCAction:
"""Get exactly one action for the provided token, and fail otherwise."""
actions = self.actions.get(token_uid)
if actions is None or len(actions) != 1:
raise NCFail(f'expected exactly 1 action for token {token_uid.hex()}')
raise NCInternalException(f'expected exactly 1 action for token {token_uid.hex()}')
return actions[0]

def copy(self) -> Context:
Expand Down
Loading
Loading