Repository navigation
fix in progress logs request - #800
Conversation
|
👋 Unheilbar, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
| return nil | ||
| } | ||
|
|
||
| const inProgressLogsLimit = 20 |
There was a problem hiding this comment.
is this supposed to be 20 or 2? From Dmytro's comment in the original thread
There was a problem hiding this comment.
basically we need to ensure we deterministically handle the case when more than 1 transmission revert happened in 1 block. Since previously we had limit 1, it wasn't clear if we get the same transaction on all the nodes in the don. This number is arbitrary, since the chance of even 2 transmission in 1 block is quite low
There was a problem hiding this comment.
Seems like with the processed event handling we no longer need to fetch multiple events. It's enough to get 1, no?
There was a problem hiding this comment.
what if eventProcessed never reached? e.g. Receiver execution contract reverted. We still want to return reverted TxHash deterministically
There was a problem hiding this comment.
We'll sort by block number, log index and txhash in LP https://github.com/smartcontractkit/chainlink-solana/blob/34d6a49c27a2b329fc71021c98dd0a4445bf698a/pkg/solana/logpoller/parser.go#L315
There was a problem hiding this comment.
If the first transaction reverts and the second succeeds, won't we return here success and sig of a reverted tx?
There was a problem hiding this comment.
Yep, updated to retreive signature from ReportProcessed log
| return nil | ||
| } | ||
|
|
||
| const inProgressLogsLimit = 20 |
There was a problem hiding this comment.
Seems like with the processed event handling we no longer need to fetch multiple events. It's enough to get 1, no?
|
3 participants
No description provided.