Fix eth_getlogs not returning error when receipt is unavailable #4056
Conversation
|
Generally fine, but I think here we are clipping to block, do we want to do that? Other small thing not sure if it should be ResourceNotFound or ResourceUnavailable? |
|
Missed the clip thing. As for the ResourceNotFound or ResourceUnavailable, it seems that other RPC that can't run when state is unavailable respond with ResourceUnavailable, but currently, if the block is not found (as in header not found, not just receipt), it returns, resourceNotFound. So.. I don't know. |
| @@ -63,18 +63,29 @@ public class LogFinder : ILogFinder | |||
| public IEnumerable<FilterLog> FindLogs(LogFilter filter, CancellationToken cancellationToken = default) | |||
| { | |||
| BlockHeader FindHeader(BlockParameter blockParameter, string name, bool headLimit) => | |||
| _blockFinder.FindHeader(blockParameter, headLimit) ?? throw new ArgumentException("Block not found.", name); | |||
| _blockFinder.FindHeader(blockParameter, headLimit) ?? throw new ResourceNotFoundException($"Block not found: {name}"); | |||
There was a problem hiding this comment.
Can we also include BlockParameter in error message?
There was a problem hiding this comment.
Seems to already do it via the name parameter.
There was a problem hiding this comment.
No its just nameof(filter.ToBlock), lets add what was that what we are missing (blocknumber, hash)
There was a problem hiding this comment.
Ok, I suppose a simple .ToString() is enough?
There was a problem hiding this comment.
yes just drop it somewhere in the pattern in some good way
asdacap
force-pushed
the
fix/eth_getlogs_unavailable_receipt_xalidation
branch
from
baf2c64
to
5584d97
Compare
Fixes #3983
Changes:
HasBlockmethod toIReceiptStorage.ReceiptPersistentStorageimplemented it by checking its cache first, then the blocks column.LogFinderwill checkHasBlockon first and last block and will throwResourceNotFoundExceptionif receipt not found.EthRpcModulewill return resource not found code whenResourceNotFoundExceptionis encountered.Types of changes
Testing
In case you checked yes, did you write tests??
Comments about testing , should you have some (optional)