Encapsulate field parsing #4
Conversation
| @@ -52,10 +35,10 @@ pub async fn init_eth_adapter(http_url: &str) -> (Provider<Http>, Contract) { | |||
| fn parse_calldata(commit_blocks_fn: &Function, calldata: &[u8]) -> Result<Vec<CommitBlockInfoV1>> { | |||
| let mut parsed_input = commit_blocks_fn | |||
| .decode_input(&calldata[4..]) | |||
| .map_err(|e| ParseError::InvalidCalldata(e.to_string()))?; | |||
| .map_err(|e| state::ParseError::InvalidCalldata(e.to_string()))?; | |||
There was a problem hiding this comment.
These shared error types here are ugly as hell and don't feel right, but let's fix these separately, if that's ok?
| impl TryFrom<&abi::Token> for CommitBlockInfoV1 { | ||
| type Error = ParseError; | ||
|
|
||
| /// Try to parse Ethereum ABI token into CommitBlockInfoV1. | ||
| /// | ||
| /// * `token` - ABI token of `CommitBlockInfo` type on Ethereum. | ||
| fn try_from(token: &abi::Token) -> Result<Self, Self::Error> { |
There was a problem hiding this comment.
I guess this is what you meant by separating the parsing?
There was a problem hiding this comment.
Precisely, this is much nicer and self-contained now!
| match CommitBlockInfoV1::try_from(data) { | ||
| Ok(blk) => res.push(blk), | ||
| Err(e) => println!("failed to parse commit block info: {}", e), | ||
| } |
There was a problem hiding this comment.
| match CommitBlockInfoV1::try_from(data) { | |
| Ok(blk) => res.push(blk), | |
| Err(e) => println!("failed to parse commit block info: {}", e), | |
| } | |
| if let Ok(block) = CommitBlockInfoV1::try_from(data) { | |
| res.push(block); | |
| } |
Don't think there's any need to print out the errors, I think we'd rather implement some tracing later down the line and log that error or even panic! since that function should only ever fail on malformed data.
There was a problem hiding this comment.
I'd rather keep this. In my experience these can easily save hours of debugging during development. Often times, some very minor unrelated mistake happens and if there's no observability, one really has to debug weird behaviour until the root cause is found. Having this kind of debug print shortens time of diagnostics a lot. Referring to earlier error handling, I'd leave panic! completely out of this code 🙂
| impl TryFrom<&abi::Token> for CommitBlockInfoV1 { | ||
| type Error = ParseError; | ||
|
|
||
| /// Try to parse Ethereum ABI token into CommitBlockInfoV1. | ||
| /// | ||
| /// * `token` - ABI token of `CommitBlockInfo` type on Ethereum. | ||
| fn try_from(token: &abi::Token) -> Result<Self, Self::Error> { |
There was a problem hiding this comment.
Precisely, this is much nicer and self-contained now!
| match decompress_bytecode(bytecode) { | ||
| Ok(bytecode) => smartcontracts.push(bytecode), | ||
| Err(e) => println!("failed to decompress bytecode: {}", e), | ||
| }; |
There was a problem hiding this comment.
| match decompress_bytecode(bytecode) { | |
| Ok(bytecode) => smartcontracts.push(bytecode), | |
| Err(e) => println!("failed to decompress bytecode: {}", e), | |
| }; | |
| if let Ok(bytecode) = decompress_bytecode(bytecode) { | |
| smartcontracts.push(bytecode); | |
| } |
Ditto
There was a problem hiding this comment.
Same as earlier. 🙂
Co-authored-by: Jonathan <[email protected]>
This PR implements
TryFromtrait forCommitBlockInfoV1and encapsulates the Eth token parsing into corresponding type.