Raise BrokenPreconditionError on invalid inputs

[pirapira]

because the caller of the proxy should have checked these inputs.

The documentation for BrokenPreconditionError clearly says

class BrokenPreconditionError(RaidenError):
    """ Raised while checking transaction preconditions
    which should be satisfied before sending the transaction.
    This exception when:
    1. An assert or a revert in the smart contract would be hit for
    triggering block.

    2. If provided values are invalid (i.e ValueError)
    """

So, when the provided values are invalid, BrokenPreconditionError
should be raised.

This PR comes from a discussion in #4936 (review)

Description

Please, describe what this PR does in detail:

• If it's a bug fix, detail the root cause of the bug and how this PR fixes it.

This is a bug fix. Before this PR, the proxy raised a recoverable error even when invalid values are passed.

PR review check list

Quality check list that cannot be automatically verified.

• Safety
  • The changes respect the necessary conditions for safety (https://raiden-network-specification.readthedocs.io/en/latest/smart_contracts.html#protocol-values-constraints)
• Code quality
  • Error conditions are handled
  • Exceptions are propagated to the correct parent greenlet
  • Exceptions are correctly classified as recoverable or unrecoverable
• Compatibility
  • State changes are forward compatible
  • Transport messages are backwards and forward compatible
• Commits
  • Have good messages
  • Squashed unecessary commits
• If it's a bug fix:
  • Regression test for the bug
    • Properly covers the bug
    • If an integration test is used, it could not be written as a unit test
• Documentation
  • A new CLI flag was introduced, is there documentation that explains usage?
• Specs
  • If this is a protocol change, are the specs updated accordingly? If so, please link PR from the specs repo.
• Is it a user facing feature/bug fix?
  • Changelog entry has been added

[karlb]

@rakanalh In #4936 (comment) you said that these should not be BrokenPreconditionErrors. Have you been convinced? It looks sensible to me.

[karlb]

Instead of removing these, you could make InvalidTokenAddress a subclass of BrokenPreconditionError. Fine grained exceptions are usually a nice thing, e.g. you could mark wrong input fields in the UI or add error codes to exceptions and pass them to API users.

[pirapira]

Will do.

[pirapira]

No, I'm not sure. When the API raises these exceptions, that should not crash the node.

Also I've seen that the proxy should not raise specific errors #4878 (comment). I'm seeing another contradiction here.

[pirapira]

@rakanalh, in the end, are proxies allowed to raise specific errors? If yes, in which circumstances? I'll document and follow what you say.

[pirapira]

@rakanalh said these should not be BrokenPreconditionError. When I read the docstring of BrokenPreconditionError, it says something like ValueError should be BrokenPreconditionError. So I'm seeing a contradiction there.

[pirapira]

On the second point, we agreed that the BrokenPreconditionError is for blockchain checks.