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

Should not execute complete shutdown on "No new blocks received for 300 seconds" #53

Open
ghost opened this issue Dec 11, 2017 · 1 comment
Labels

Comments

@ghost
Copy link

ghost commented Dec 11, 2017

Whenever the "No new blocks received for 300 seconds, the keeper will terminate" occurs, the full graceful shutdown takes place now. It might not be the desired thing though, as the full shutdown procedure may involve for example cancelling orders on Oasis etc. But if we start cancelling them when the chain is not in sync, we have no way to see if our transactions got properly mined or not. In case of increasing gas price it will make the keeper unnecessarily raise the gas price too high.

You can see what I mean in the following example. The node stopped syncing for about half an hour, the market-maker-keeper detected it and started to cancel orders. But the cancellation transactions probably did not even propagate, but it kept raising the gas price which was a complete waste as the transactions didn't get mined until the node started syncing again:

2017-12-11 07:10:26,952 INFO     Keeper terminated
2017-12-11 07:10:26,952 INFO     Shutdown logic finished
2017-12-11 07:10:26,951 INFO     Transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) was successful (tx_hash=0x86...)
2017-12-11 06:42:03,122 INFO     Sent transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) with nonce=7432, gas=214207, gas_price=150000000000 (tx_hash=0x86...)
2017-12-11 06:41:03,200 INFO     Sent transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) with nonce=7432, gas=214207, gas_price=130000000000 (tx_hash=0x57...)
2017-12-11 06:40:03,265 INFO     Sent transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) with nonce=7432, gas=214207, gas_price=110000000000 (tx_hash=0x2b...)
2017-12-11 06:39:03,293 INFO     Sent transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) with nonce=7432, gas=214207, gas_price=90000000000 (tx_hash=0x5c...)
2017-12-11 06:38:03,280 INFO     Sent transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) with nonce=7432, gas=214207, gas_price=70000000000 (tx_hash=0xc5...)
2017-12-11 06:37:03,781 INFO     Sent transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) with nonce=7432, gas=214207, gas_price=50000000000 (tx_hash=0x60...)
2017-12-11 06:37:02,127 INFO     Executing keeper shutdown logic...
2017-12-11 06:37:02,126 INFO     Waiting for outstanding callback to terminate...
2017-12-11 06:37:01,389 INFO     Waiting for all threads to terminate...
2017-12-11 06:37:01,389 INFO     Shutting down the keeper
2017-12-11 06:37:01,389 CRITICAL No new blocks received for 300 seconds, the keeper will terminate

Ultimately, I think the keeper should have a choice how to configure the lifecycle. It should be able to bind to an emergency_shutdown event.

@ghost ghost added the bug label Dec 11, 2017
@EdNoepel
Copy link
Contributor

This is painfully apparent when working on a local testchain. My use case was integration testing auction-keeper. Workaround is to run a script which records a meaningless transaction (wrapping 1ETH) every 4 minutes.

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

No branches or pull requests

1 participant