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

Added contract::call_with_confirmations(). #102

Merged
merged 3 commits into from
Mar 15, 2018

Conversation

akuanti
Copy link
Contributor

@akuanti akuanti commented Mar 7, 2018

Allows you to call a contract function and wait for a specified number of confirmations.

Copy link
Owner

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Looks good apart from an unhandled error.


let fn_data = self.abi.function(func.into())
.and_then(|function| function.encode_input(&params.into_tokens()))
.unwrap();
Copy link
Owner

Choose a reason for hiding this comment

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

Would be good to handle the error here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what is the right way to handle this (still getting used to Rust). Should the function return a Result?

Copy link
Owner

Choose a reason for hiding this comment

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

Have a look at:

impl<T, F, E> From<E> for QueryResult<T, F> where
E: Into<contract::Error>,
{
fn from(e: E) -> Self {
QueryResult {
inner: ResultType::Constant(Err(e.into()))
}
}
}

CallResult in that case encapsulate an asynchronous result (positive Ok() and negative Err()).

We should probably have something similar for SendTransactionWithConfirmation type.
It will require introducing new state here:

enum SendTransactionWithConfirmationState<T: Transport> {

that immediately resolves to an error.

Copy link
Owner

Choose a reason for hiding this comment

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

Let me know if you need more help with 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.

Yeah, I think I do need more help with this. Are you saying that call_with_confirmations() should still return SendTransactionWithConfirmation, but that SendTransactionWithConfirmation should have an error state so that any errors that happen in the function can be converted into it?

Copy link
Owner

Choose a reason for hiding this comment

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

@akuanti I've made a PR to your branch to patch that. Although I'm not super happy with the solution,cause the error types does not match. We can cover that as part of #103 though.

@tomusdrw tomusdrw merged commit 1725c3d into tomusdrw:master Mar 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants