Return Result from methods of ChainAdapter

[hashmap]

Most of the methods return nothing or bool which is used to decide if a
sender of a message should be banned or not. However underlying chain
implementation may fail so we need a way to reflect this fact in API.

Also it allows to reduce number of unwraps and makes the code more robust.

[JeremyRubin]

Concept Ack.

I'm not in love with introducing a dependency from p2p to chain module for two reasons:

1. what if these methods should return a different error type in the future
2. it introduces a new dependency on a bunch of other stuff (including the keychain module)

Is there an alternative approach -- can we move chain's error type to it's own module or maybe provide a module for cross-module errors (containing a enum with a variant for each module, whereas the From<MyModuleError> for AllModulesError wraps the serialized error in the appropriate variant?

[hashmap]

@JeremyRubin I had the same feelings and tried different approaches, but haven't find anything more reasonable than just use chain::Error. My main rationale that the code must be candid.

We deal with ChainAdapter, so we should accept the fact that we have a dependency on chain crate. The methods should return chain::Error for the same reason. Actually I changed the trait first and then worked with implementations, so it was a design decision.

I think we should tackle error management in a more holistic way, I created an issue to tack it a while ago #2542 and I had the same idea of error crate to rule them all. To be fair I don't think it's a right approach anymore, but not sure.

[garyyu]

@hashmap

I think we should tackle error management in a more holistic way, I created an issue to tack it a while ago #2542 and I had the same idea of error crate to rule them all.
👍

To be fair I don't think it's a right approach anymore, but not sure.

what's this it? ^
#2542 or this PR?

[hashmap]

@garyyu the idea to have a designated error crate I proposed as an option in #2542

[ignopeverell]

Mind fixing conflicts? I agree with the approach.