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.

Sign up for GitHub

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

Handle exception when getting block timestamp #792

Merged
merged 6 commits into from
Jan 17, 2024
Merged

Handle exception when getting block timestamp #792

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8eebb0d
Handle exception when getting block timestamp
trizin Jan 17, 2024
b3feb59
Return 0 on error
trizin Jan 17, 2024
db30b3a
Test error handling
trizin Jan 17, 2024
0116c31
Prettier naming
trizin Jan 17, 2024
ae9f282
Fix order
trizin Jan 17, 2024
3ee65e5
Fix test
trizin Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion df_py/util/blocktime.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,11 @@ def __init__(self, target_timestamp):
self.target_timestamp = target_timestamp

def timeSinceTimestamp(self, block_i):
block_timestamp = web3.eth.get_block(int(block_i)).timestamp
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
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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:

  1. Write a unit test that fails because it's working the bug
  2. Write code to make the test pass
  3. See the test pass, done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe if there's an error, return a value of 0, rather than - self.target_timestamp ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, committed, will proceed with testing. Thanks!

return block_timestamp - self.target_timestamp

f = C(timestamp).timeSinceTimestamp
Expand Down
Loading