Handle exception when getting block timestamp #792
Conversation
df_py/util/blocktime.py
Outdated
| try: | ||
| block_timestamp = web3.eth.get_block(int(block_i)).timestamp | ||
| except Exception as e: | ||
| print(f"An exception occurred while getting block {block_i}, {e}") | ||
| block_timestamp = 0 |
There was a problem hiding this comment.
So if the exception happens, then block_timestamp = 0
And it returns block_timestamp - self.target_timestamp = 0 - self.target_timestamp = -self.target_timestamp.
This seems wrong.
There was a problem hiding this comment.
One more thing: we should have a unit test for this exception too.
In general, when fixing a bug the best flow is to do the following:
- Write a unit test that fails because it's working the bug
- Write code to make the test pass
- See the test pass, done
There was a problem hiding this comment.
Based on my understanding, the Sapphire RPC error occurs when the block number is too far in the past, causing the active flow to fail. The proposed solution to address this issue involves setting the timestamp to 0 if this error is encountered, under the assumption that the block number is very much in the past.
Failed flow:
https://github.com/oceanprotocol/df-py/actions/runs/7555123620/job/20569794417?pr=790
I've tried running this solution locally for the previous week and the algorithm was able to find the correct timestamps for week start and week end.
Before proceeding with testing, I would appreciate your feedback on whether you think there might be a more optimal approach or if this solution is acceptable.
There was a problem hiding this comment.
Maybe if there's an error, return a value of 0, rather than - self.target_timestamp ?
There was a problem hiding this comment.
Makes sense, committed, will proceed with testing. Thanks!
|
Code Climate has analyzed commit db30b3a and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 40.0% (90% is the threshold). This pull request will bring the total coverage in the repository to 92.9% (0.0% change). View more on Code Climate. |
trizin
deleted the
issue791-handle-block-not-found-errors-in-timestamp_to_block
branch
Fixes #791
Changes proposed in this PR: