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

Add timeout for each transaction #2424

Closed
4 tasks
xiaoxiaff opened this issue Oct 1, 2018 · 6 comments
Closed
4 tasks

Add timeout for each transaction #2424

xiaoxiaff opened this issue Oct 1, 2018 · 6 comments

Comments

@xiaoxiaff
Copy link
Contributor

Summary

Add timeout for each transaction.

Problem Definition

For now if a transaction runs for more than one block time it will result in failed to propose a block. Even more if there exists some bugs which causes the transaction running for a long time, that node will deny all incoming service.

Proposal

Wrap runTx with timeout so it will not failed when transaction runs into timeout situation.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@Thunnini
Copy link
Contributor

Thunnini commented Oct 2, 2018

This approach is not deterministic, and will chain halt if each node produces different results for the timeout? I think that solution is to set the gas well depending on the transaction's processing...

@ValarDragon
Copy link
Contributor

ValarDragon commented Oct 2, 2018

As @Thunnini this is the point of gas. In the event of an infinite loop in the code (i.e. tendermint/go-amino#197) I think we should halt. If we keep on going, this will cause problems for anyone who full syncs for the rest of time. (Such an attack will likely be executed every block)

I'm pretty opposed to this due to the inherent non-determinism, and troubles it will cause for full syncing though. Within the SDK I'm not that concerned with infinite loops, since we have gas costs per iterator which will halt the iterator. I think it is reasonable to expect the imported libraries to prevent the possibility of infinite loops. (Though perhaps this is unfounded given the above issue I cited)

@cwgoes
Copy link
Contributor

cwgoes commented Oct 2, 2018

We can't implement any kind of clock-time-based logic SDK side, since it isn't in consensus and will vary according to machine resources.

Proposers can elect to simulate transactions before proposing them and apply their own arbitrary time limit, simply refusing to include transactions which time out. Will that address your concern?

@xiaoxiaff
Copy link
Contributor Author

We can't implement any kind of clock-time-based logic SDK side, since it isn't in consensus and will vary according to machine resources.

Agree with that, but actually each node have a clock-time in Tendermint (timeout_commit), if a tx running longer than block commit time, this node has to terminate that tx. If it doesn't terminate it will still failed to catch that round.

For the reason I proposal this is we have a case that we used big.Rat.SetString() in our code and it will timeout if pass in a really large number (like 10e99999999999). A grace timeout mechanism may help with this kind of situation.

@xiaoxiaff
Copy link
Contributor Author

If you guys think this is better to be implemented in Tendermint I am totally agree :)

@jackzampolin
Copy link
Member

Sounds like this proposal is infeasible to implement in the SDK. Closing this issue. If you thing we should continue the discussion, please open this issue on tendermint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants