feat: add retry to get_receipts#1847
Conversation
KolbyML
force-pushed
the
retry-for-Receipt
branch
from
21a29a5 to
9f73576
Compare
| Ok((content_key, content_value)) | ||
| } | ||
|
|
||
| /// This function should only be used if we know the receipts should exist. |
There was a problem hiding this comment.
Can you clarify what you mean by: "if we know the receipts should exist"?
It implies that somebody might use it when either:
- they know it doesn't exist (in which case, why would they call it in the first place?)
- they are not sure if it exists. How can this happen? What should they do instead?
nit: somewhat less important, the first line of the function documentation usually stats on high level what function does, and the other lines describe more detailed behaviour
There was a problem hiding this comment.
Ok I updated the documentation, to make it more clear
| /// This function should only be used if we know the receipts should exist. | ||
| /// | ||
| /// The function will retry 5 times if the receipts are not ready yet. | ||
| /// Especially when querying for receipts at the end of the chain their is a delay until they |
There was a problem hiding this comment.
| /// Especially when querying for receipts at the end of the chain their is a delay until they | |
| /// Especially when querying for receipts at the head of the chain, there is a delay until they |
| /// The function will retry 5 times if the receipts are not ready yet. | ||
| /// Especially when querying for receipts at the end of the chain their is a delay until they | ||
| /// become available. | ||
| pub async fn get_receipts(&self, block_number: u64) -> anyhow::Result<Receipts> { |
There was a problem hiding this comment.
nit: would it make sense to have two functions:
get_receipts- without retry logicget_receipts_retry- with retry logic, that would callget_receipts
Up to you
There was a problem hiding this comment.
I think this method is inherently more reliable so there is only upside's in all our usages. For example in the e2hs-writer head bridge I seen it fail to get receipts once, so it is fairly rare, but an EL can return null for any reason
There was a problem hiding this comment.
So I will just keep it as 1
2 participants
This PR is a requirement for
What was wrong?
There is a delay for when receipts become available, as the EL has to generate them. I add something similar to this code because we needed it to make the latest bridge reliable. Due to the aforementioned issue above. I removed this logic in this PR #1757 well optimizing the Single-E2HS-Writer, because we no longer used this functionality.
How was it fixed?
Added the retry mechanism back, but only for
get_receipts(), because it isn't needed inget_receipts_rangeas that function doesn't really have a reason to be used at the head of the chain.I choose 5, because as I have seen delays of 3 seconds