Repository: raiden-network/raiden-services

Issue:
`claim_iou` script doesn't check if expiration is in the past
The `claim_iou` script doesn't properly check if an IOUs expiration is in the past.

This leads to gas estimation to fail and the script to crash.
```
[info     ] Opening database               [raiden_libs.database] filename=/state/pfs-state.db
Found 1 claimable IOUs
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/local/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/services/src/pathfinding_service/claim_fees.py", line 135, in <module>
    main(auto_envvar_prefix="PFS")  # pragma: no cover
  File "/usr/local/lib/python3.7/site-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.7/site-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.7/site-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/services/src/raiden_libs/cli.py", line 243, in call_with_blockchain_info
    return callback(**params)
  File "/services/src/raiden_libs/cli.py", line 168, in call_with_common_options_initialized
    return func(**params)
  File "/services/src/pathfinding_service/claim_fees.py", line 63, in main
    _, failures = claim_ious(ious, claim_cost_rdn, contracts[CONTRACT_ONE_TO_N], web3, database)
  File "/services/src/pathfinding_service/claim_fees.py", line 111, in claim_ious
    tx_hash = claim.transact()
  File "/usr/local/lib/python3.7/site-packages/web3/contract.py", line 996, in transact
    **self.kwargs
  File "/usr/local/lib/python3.7/site-packages/web3/contract.py", line 1580, in transact_with_contract_function
    txn_hash = web3.eth.sendTransaction(transact_transaction)
  File "/usr/local/lib/python3.7/site-packages/web3/eth.py", line 381, in sendTransaction
    get_buffered_gas_estimate(self.web3, transaction),
  File "/usr/local/lib/python3.7/site-packages/web3/_utils/transactions.py", line 126, in get_buffered_gas_estimate
    gas_estimate = web3.eth.estimateGas(gas_estimate_transaction)
  File "/usr/local/lib/python3.7/site-packages/web3/eth.py", line 445, in estimateGas
    params,
  File "/usr/local/lib/python3.7/site-packages/web3/manager.py", line 153, in request_blocking
    raise ValueError(response["error"])
ValueError: {'code': -32000, 'message': 'gas required exceeds allowance (9983436) or always failing transaction'}
```

While at it, we should also add an explanation on how to run the script on the RSB.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/raiden-network/raiden-services/pull/790?src=pr&el=h1) Report
> Merging [#790](https://codecov.io/gh/raiden-network/raiden-services/pull/790?src=pr&el=desc) into [master](https://codecov.io/gh/raiden-network/raiden-services/commit/ac396cd7eb1687b84bb479acfc08b667f5bcd955&el=desc) will **decrease** coverage by `0.06%`.
> The diff coverage is `0.00%`.

[![Impacted file tree graph](https://codecov.io/gh/raiden-network/raiden-services/pull/790/graphs/tree.svg?width=650&height=150&src=pr&token=H07qtpFA6p)](https://codecov.io/gh/raiden-network/raiden-services/pull/790?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #790      +/-   ##
==========================================
- Coverage   89.80%   89.74%   -0.07%     
==========================================
  Files          35       35              
  Lines        2639     2643       +4     
  Branches      334      335       +1     
==========================================
+ Hits         2370     2372       +2     
- Misses        199      200       +1     
- Partials       70       71       +1     
```


| [Impacted Files](https://codecov.io/gh/raiden-network/raiden-services/pull/790?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/pathfinding\_service/claim\_fees.py](https://codecov.io/gh/raiden-network/raiden-services/pull/790/diff?src=pr&el=tree#diff-c3JjL3BhdGhmaW5kaW5nX3NlcnZpY2UvY2xhaW1fZmVlcy5weQ==) | `84.93% <0.00%> (-4.93%)` | :arrow_down: |
| [src/pathfinding\_service/api.py](https://codecov.io/gh/raiden-network/raiden-services/pull/790/diff?src=pr&el=tree#diff-c3JjL3BhdGhmaW5kaW5nX3NlcnZpY2UvYXBpLnB5) | `95.68% <0.00%> (+0.78%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/raiden-network/raiden-services/pull/790?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/raiden-network/raiden-services/pull/790?src=pr&el=footer). Last update [ac396cd...aa41053](https://codecov.io/gh/raiden-network/raiden-services/pull/790?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/pathfinding_service/claim_fees.py b/src/pathfinding_service/claim_fees.py
index b368084..6f56178 100644
--- a/src/pathfinding_service/claim_fees.py
+++ b/src/pathfinding_service/claim_fees.py
@@ -55,6 +55,7 @@ def main(
     ious = list(
         get_claimable_ious(
             database,
+            expires_after=web3.eth.blockNumber,
             expires_before=BlockNumber(web3.eth.blockNumber + expires_within),
             claim_cost_rdn=claim_cost_rdn,
         )
@@ -77,12 +78,17 @@ def calc_claim_cost_rdn(web3: Web3, rdn_per_eth: float) -> TokenAmount:
 
 
 def get_claimable_ious(
-    database: PFSDatabase, expires_before: BlockNumber, claim_cost_rdn: TokenAmount
+    database: PFSDatabase,
+    expires_after: BlockNumber,
+    expires_before: BlockNumber,
+    claim_cost_rdn: TokenAmount,
 ) -> Iterable[IOU]:
-    ious = database.get_ious(
-        claimed=False, expires_before=expires_before, amount_at_least=claim_cost_rdn
+    return database.get_ious(
+        claimed=False,
+        expires_after=expires_after,
+        expires_before=expires_before,
+        amount_at_least=claim_cost_rdn,
     )
-    return ious
 
 
 def claim_ious(
diff --git a/src/pathfinding_service/database.py b/src/pathfinding_service/database.py
index 8762480..68055b2 100644
--- a/src/pathfinding_service/database.py
+++ b/src/pathfinding_service/database.py
@@ -115,6 +115,7 @@ class PFSDatabase(BaseDatabase):
         sender: Optional[Address] = None,
         expiration_block: Optional[BlockNumber] = None,
         claimed: Optional[bool] = None,
+        expires_after: Optional[BlockNumber] = None,
         expires_before: Optional[BlockNumber] = None,
         amount_at_least: Optional[TokenAmount] = None,
     ) -> Iterator[IOU]:
@@ -136,6 +137,9 @@ class PFSDatabase(BaseDatabase):
         if expires_before is not None:
             query += " AND expiration_block < ?"
             args.append(hex256(expires_before))
+        if expires_after is not None:
+            query += " AND expiration_block > ?"
+            args.append(hex256(expires_after))
         if amount_at_least is not None:
             query += " AND amount >= ?"
             args.append(hex256(amount_at_least))


Test patch:
diff --git a/tests/pathfinding/test_claim_fees.py b/tests/pathfinding/test_claim_fees.py
index 4039a3d..1382565 100644
--- a/tests/pathfinding/test_claim_fees.py
+++ b/tests/pathfinding/test_claim_fees.py
@@ -5,6 +5,7 @@ import pytest
 from click.testing import CliRunner
 from eth_utils import decode_hex, to_canonical_address
 from tests.libs.mocks.web3 import Web3Mock
+from web3 import Web3
 
 from pathfinding_service.claim_fees import claim_ious, get_claimable_ious, main
 from pathfinding_service.model import IOU
@@ -16,7 +17,7 @@ from raiden.utils.typing import BlockNumber, ChainID, Signature, TokenAmount
 def test_claim_fees(  # pylint: disable=too-many-locals
     pathfinding_service_web3_mock: PathfindingService,
     one_to_n_contract,
-    web3,
+    web3: Web3,
     deposit_to_udc,
     get_accounts,
     get_private_key,
@@ -24,7 +25,7 @@ def test_claim_fees(  # pylint: disable=too-many-locals
     pfs = pathfinding_service_web3_mock
 
     # Prepare test data
-    accounts = [decode_hex(acc) for acc in get_accounts(6)]
+    accounts = [decode_hex(acc) for acc in get_accounts(7)]
     iou_inputs: List[dict] = [
         dict(sender=accounts[0], amount=100, deposit=200),
         dict(sender=accounts[1], amount=200, deposit=100),
@@ -33,6 +34,9 @@ def test_claim_fees(  # pylint: disable=too-many-locals
         dict(sender=accounts[4], amount=104, claimed=True),  # already claimed
         dict(sender=accounts[4], amount=99),  # too low amount
         dict(sender=accounts[5], expiration_block=1000, amount=104),  # does not expire, yet
+        dict(
+            sender=accounts[6], expiration_block=web3.eth.blockNumber - 1, amount=104
+        ),  # already expired
     ]
 
     # Create IOUs from `iou_inputs`
@@ -60,6 +64,7 @@ def test_claim_fees(  # pylint: disable=too-many-locals
     claimable_ious = list(
         get_claimable_ious(
             database=pfs.database,
+            expires_after=web3.eth.blockNumber,
             expires_before=BlockNumber(1000),
             claim_cost_rdn=TokenAmount(100),
         )