Skip to content
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.

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

Unconfirmed blocks #502

Merged
merged 7 commits into from
Aug 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion src/monitoring_service/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
MAX_FILTER_INTERVAL: int = 100_000
DEFAULT_GAS_BUFFER_FACTOR: int = 10
DEFAULT_GAS_CHECK_BLOCKS: int = 100
DEFAULT_PAYMENT_RISK_FAKTOR: int = 2
KEEP_MRS_WITHOUT_CHANNEL: timedelta = timedelta(minutes=15)

# A LockedTransfer message is roughly 1kb. Having 1000/min = 17/sec will be
Expand Down
6 changes: 3 additions & 3 deletions src/monitoring_service/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,22 @@
from raiden_libs.events import Event


@dataclass
@dataclass(frozen=True)
class ScheduledEvent(Event):
""" An event to be triggered a t a certain block number. """

trigger_block_number: BlockNumber
event: Event


@dataclass
@dataclass(frozen=True)
class ActionMonitoringTriggeredEvent(Event):
token_network_address: TokenNetworkAddress
channel_identifier: ChannelID
non_closing_participant: Address


@dataclass
@dataclass(frozen=True)
class ActionClaimRewardTriggeredEvent(Event):
token_network_address: TokenNetworkAddress
channel_identifier: ChannelID
Expand Down
117 changes: 69 additions & 48 deletions src/monitoring_service/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from web3 import Web3
from web3.contract import Contract

from monitoring_service.constants import DEFAULT_PAYMENT_RISK_FAKTOR
from monitoring_service.database import Database
from monitoring_service.events import (
ActionClaimRewardTriggeredEvent,
Expand All @@ -21,6 +20,8 @@
)
from raiden.utils.typing import BlockNumber, TokenNetworkAddress, TransactionHash
from raiden_contracts.constants import CONTRACT_TOKEN_NETWORK, ChannelState
from raiden_libs.blockchain import get_pessimistic_udc_balance
from raiden_libs.constants import UDC_SECURITY_MARGIN_FACTOR
from raiden_libs.contract_info import CONTRACT_MANAGER
from raiden_libs.events import (
Event,
Expand All @@ -41,10 +42,14 @@ class Context:
ms_state: MonitoringServiceState
db: Database
w3: Web3
last_known_block: int
monitoring_service_contract: Contract
user_deposit_contract: Contract
min_reward: int
required_confirmations: int

@property
def latest_known_block(self) -> BlockNumber:
return self.ms_state.blockchain_state.latest_known_block


def channel_opened_event_handler(event: Event, context: Context) -> None:
Expand Down Expand Up @@ -108,7 +113,7 @@ def channel_closed_event_handler(event: Event, context: Context) -> None:
# Check if the settle timeout is already over.
# This is important when starting up the MS.
settle_period_end_block = event.block_number + channel.settle_timeout
settle_period_over = settle_period_end_block < context.last_known_block
settle_period_over = settle_period_end_block < context.latest_known_block
if not settle_period_over:
# Trigger the monitoring action event handler, this will check if a
# valid MR is available.
Expand Down Expand Up @@ -152,7 +157,7 @@ def channel_closed_event_handler(event: Event, context: Context) -> None:
token_network_address=event.token_network_address,
identifier=channel.identifier,
settle_period_end_block=settle_period_end_block,
known_block=context.last_known_block,
known_block=context.latest_known_block,
)

channel.state = ChannelState.CLOSED
Expand Down Expand Up @@ -406,67 +411,83 @@ def action_monitoring_triggered_event_handler(event: Event, context: Context) ->
if channel.update_status:
last_onchain_nonce = channel.update_status.nonce

user_address = monitor_request.non_closing_signer
user_deposit = context.user_deposit_contract.functions.effectiveBalance(user_address).call()

if monitor_request.nonce <= last_onchain_nonce:
log.info(
"Another MS submitted the last known channel state", monitor_request=monitor_request
)
return

latest_block = context.w3.eth.blockNumber
last_confirmed_block = latest_block - context.required_confirmations
karlb marked this conversation as resolved.
Show resolved Hide resolved
user_address = monitor_request.non_closing_signer
user_deposit = get_pessimistic_udc_balance(
udc=context.user_deposit_contract,
address=user_address,
from_block=last_confirmed_block,
to_block=latest_block,
)
if monitor_request.reward_amount < context.min_reward:
log.info(
"Monitor request not executed due to insufficient reward amount",
monitor_request=monitor_request,
min_reward=context.min_reward,
)
return

call_monitor = (
if user_deposit < monitor_request.reward_amount * UDC_SECURITY_MARGIN_FACTOR:
log.debug(
"User deposit is insufficient -> try monitoring again later",
monitor_request=monitor_request,
min_reward=context.min_reward,
)
context.db.upsert_scheduled_event(
ScheduledEvent(trigger_block_number=BlockNumber(last_confirmed_block + 1), event=event)
)
return

assert (
channel.closing_tx_hash is None
and monitor_request.nonce > last_onchain_nonce
and user_deposit >= monitor_request.reward_amount * DEFAULT_PAYMENT_RISK_FAKTOR
and monitor_request.reward_amount >= context.min_reward
)
if call_monitor:
try:
# Attackers might be able to construct MRs that make this fail.
# Since we execute a gas estimation before doing the `transact`,
# the gas estimation will fail before any gas is used.
# If we stop doing a gas estimation, a `call` has to be done before
# the `transact` to prevent attackers from wasting the MS's gas.
tx_hash = TransactionHash(
bytes(
context.monitoring_service_contract.functions.monitor(
monitor_request.signer,
monitor_request.non_closing_signer,
monitor_request.balance_hash,
monitor_request.nonce,
monitor_request.additional_hash,
monitor_request.closing_signature,
monitor_request.non_closing_signature,
monitor_request.reward_amount,
monitor_request.token_network_address,
monitor_request.reward_proof_signature,
).transact({"from": context.ms_state.address})
)
), "This MS already monitored this channel. Should be impossible."
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be triggered somehow? Like:

  • MS has a BP, and call monitor with it, thus closing_tx_hash is set
  • We receive a new MonitorRequest
  • Monitoring is triggered again

It's quite constructed because the client would need to be online...

Copy link
Contributor Author

@karlb karlb Aug 8, 2019

Choose a reason for hiding this comment

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

If we receive a new MR after calling monitor, should we call monitor again with the updated BP? That would increase our gas cost. But otherwise a different MS might do that and we will get no reward, although we already have spent gas.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess in general this scenario is not likely.

And what you describe sounds like a loose/loose situation, not sure how we get out of that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #504.


try:
# Attackers might be able to construct MRs that make this fail.
# Since we execute a gas estimation before doing the `transact`,
# the gas estimation will fail before any gas is used.
# If we stop doing a gas estimation, a `call` has to be done before
# the `transact` to prevent attackers from wasting the MS's gas.
tx_hash = TransactionHash(
bytes(
context.monitoring_service_contract.functions.monitor(
monitor_request.signer,
monitor_request.non_closing_signer,
monitor_request.balance_hash,
monitor_request.nonce,
monitor_request.additional_hash,
monitor_request.closing_signature,
monitor_request.non_closing_signature,
monitor_request.reward_amount,
monitor_request.token_network_address,
monitor_request.reward_proof_signature,
).transact({"from": context.ms_state.address})
)
)

log.info(
"Sent transaction calling `monitor` for channel",
token_network_address=channel.token_network_address,
channel_identifier=channel.identifier,
transaction_hash=encode_hex(tx_hash),
)
assert tx_hash is not None
log.info(
"Sent transaction calling `monitor` for channel",
token_network_address=channel.token_network_address,
channel_identifier=channel.identifier,
transaction_hash=encode_hex(tx_hash),
)
assert tx_hash is not None

with context.db.conn:
# Add tx hash to list of waiting transactions
context.db.add_waiting_transaction(tx_hash)
with context.db.conn:
# Add tx hash to list of waiting transactions
context.db.add_waiting_transaction(tx_hash)

channel.closing_tx_hash = tx_hash
context.db.upsert_channel(channel)
except Exception as exc: # pylint: disable=broad-except
log.error("Sending tx failed", exc_info=True, err=exc)
channel.closing_tx_hash = tx_hash
context.db.upsert_channel(channel)
except Exception as exc: # pylint: disable=broad-except
log.error("Sending tx failed", exc_info=True, err=exc)


def action_claim_reward_triggered_event_handler(event: Event, context: Context) -> None:
Expand Down
4 changes: 1 addition & 3 deletions src/monitoring_service/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ def __init__( # pylint: disable=too-many-arguments
ms_state=ms_state,
db=self.database,
w3=self.web3,
last_known_block=0,
monitoring_service_contract=monitoring_contract,
user_deposit_contract=user_deposit_contract,
min_reward=min_reward,
required_confirmations=required_confirmations,
)

def start(
Expand Down Expand Up @@ -133,8 +133,6 @@ def start(
sys.exit(0)

def _process_new_blocks(self, last_block: BlockNumber) -> None:
self.context.last_known_block = last_block

# BCL return a new state and events related to channel lifecycle
new_chain_state, events = get_blockchain_events(
web3=self.web3,
Expand Down
13 changes: 10 additions & 3 deletions src/pathfinding_service/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from web3 import Web3

import pathfinding_service.exceptions as exceptions
from pathfinding_service.config import (
from pathfinding_service.constants import (
API_PATH,
DEFAULT_API_HOST,
DEFAULT_API_PORT,
Expand All @@ -26,7 +26,6 @@
MAX_AGE_OF_IOU_REQUESTS,
MAX_PATHS_PER_REQUEST,
MIN_IOU_EXPIRY,
UDC_SECURITY_MARGIN_FACTOR,
)
from pathfinding_service.model import IOU
from pathfinding_service.model.feedback import FeedbackToken
Expand All @@ -35,6 +34,8 @@
from raiden.exceptions import InvalidSignature
from raiden.utils.signer import recover
from raiden.utils.typing import Address, PaymentAmount, Signature, TokenAmount, TokenNetworkAddress
from raiden_libs.blockchain import get_pessimistic_udc_balance
from raiden_libs.constants import UDC_SECURITY_MARGIN_FACTOR
from raiden_libs.marshmallow import ChecksumAddress, HexedBytes

log = structlog.get_logger(__name__)
Expand Down Expand Up @@ -230,7 +231,13 @@ def process_payment( # pylint: disable=too-many-branches

# Check client's deposit in UserDeposit contract
udc = pathfinding_service.user_deposit_contract
udc_balance = udc.functions.effectiveBalance(iou.sender).call()
latest_block = pathfinding_service.web3.eth.blockNumber
udc_balance = get_pessimistic_udc_balance(
udc=udc,
address=iou.sender,
from_block=latest_block - pathfinding_service.required_confirmations,
to_block=latest_block,
)
required_deposit = round(expected_amount * UDC_SECURITY_MARGIN_FACTOR)
if udc_balance < required_deposit:
raise exceptions.DepositTooLow(required_deposit=required_deposit)
Expand Down
2 changes: 1 addition & 1 deletion src/pathfinding_service/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from web3.contract import Contract

from pathfinding_service.api import ServiceApi
from pathfinding_service.config import (
from pathfinding_service.constants import (
DEFAULT_API_HOST,
DEFAULT_INFO_MESSAGE,
DEFAULT_POLL_INTERVALL,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,3 @@

MAX_AGE_OF_IOU_REQUESTS: timedelta = timedelta(hours=1)
MAX_AGE_OF_FEEDBACK_REQUESTS: timedelta = timedelta(minutes=10)

# Since the UDC deposits are not double spend safe, you want a higher deposit
# than you're able to claim to reduce the possibility of double spends.
UDC_SECURITY_MARGIN_FACTOR: float = 2
2 changes: 1 addition & 1 deletion src/pathfinding_service/model/channel_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from eth_utils import to_checksum_address
from marshmallow_dataclass import add_schema

from pathfinding_service.config import DEFAULT_REVEAL_TIMEOUT
from pathfinding_service.constants import DEFAULT_REVEAL_TIMEOUT
from pathfinding_service.exceptions import InvalidPFSFeeUpdate
from raiden.transfer.mediated_transfer.mediation_fee import FeeScheduleState as FeeScheduleRaiden
from raiden.utils.typing import (
Expand Down
2 changes: 1 addition & 1 deletion src/pathfinding_service/model/feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from datetime import datetime
from uuid import UUID, uuid4

from pathfinding_service.config import MAX_AGE_OF_FEEDBACK_REQUESTS
from pathfinding_service.constants import MAX_AGE_OF_FEEDBACK_REQUESTS
from raiden.utils.typing import TokenNetworkAddress


Expand Down
2 changes: 1 addition & 1 deletion src/pathfinding_service/model/token_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from eth_utils import to_checksum_address
from networkx import DiGraph

from pathfinding_service.config import (
from pathfinding_service.constants import (
DEFAULT_SETTLE_TO_REVEAL_TIMEOUT_RATIO,
DIVERSITY_PEN_DEFAULT,
FEE_PEN_DEFAULT,
Expand Down
4 changes: 2 additions & 2 deletions src/pathfinding_service/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def __init__( # pylint: disable=too-many-arguments
self.user_deposit_contract = contracts[CONTRACT_USER_DEPOSIT]
self.chain_id = ChainID(int(web3.net.version))
self.address = private_key_to_address(private_key)
self._required_confirmations = required_confirmations
self.required_confirmations = required_confirmations
self._poll_interval = poll_interval
self._is_running = gevent.event.Event()

Expand Down Expand Up @@ -128,7 +128,7 @@ def _run(self) -> None: # pylint: disable=method-hidden
start_block=self.database.get_latest_known_block(),
)
while not self._is_running.is_set():
last_confirmed_block = self.web3.eth.blockNumber - self._required_confirmations
last_confirmed_block = self.web3.eth.blockNumber - self.required_confirmations

max_query_interval_end_block = (
self.database.get_latest_known_block() + MAX_FILTER_INTERVAL
Expand Down
17 changes: 15 additions & 2 deletions src/raiden_libs/blockchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
from eth_utils import decode_hex, encode_hex, to_checksum_address
from eth_utils.abi import event_abi_to_log_topic
from web3 import Web3
from web3.contract import get_event_data
from web3.contract import Contract, get_event_data
from web3.utils.abi import filter_by_type

from raiden.utils.typing import Address, BlockNumber
from raiden.utils.typing import Address, BlockNumber, TokenAmount
from raiden_contracts.constants import (
CONTRACT_MONITORING_SERVICE,
CONTRACT_TOKEN_NETWORK,
Expand Down Expand Up @@ -238,3 +238,16 @@ def get_monitoring_blockchain_events(
)

return events


def get_pessimistic_udc_balance(
udc: Contract, address: Address, from_block: BlockNumber, to_block: BlockNumber
) -> TokenAmount:
""" Get the effective UDC balance using the block with the lowest result.

Blocks between the latest confirmed block and the latest block are considered.
"""
return min(
udc.functions.effectiveBalance(address).call(block_identifier=block)
for block in range(from_block, to_block + 1)
)
3 changes: 3 additions & 0 deletions src/raiden_libs/constants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Since the UDC deposits are not double spend safe, you want a higher deposit
# than you're able to claim to reduce the possibility of double spends.
UDC_SECURITY_MARGIN_FACTOR: float = 2
1 change: 1 addition & 0 deletions tests/libs/mocks/web3.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class Web3Mock(Mock):
def __init__(self, **kwargs):
super().__init__(**kwargs)
self.net.version = 1 # chain_id
self.eth.blockNumber = 100

def _get_child_mock(self, **kwargs):
return Mock(**kwargs)
Loading