fix: wait on solana confirmed tx #11515
Conversation
AdamSchinzel
requested review from
alex-jerechinsky,
tomasklim,
matejkriz,
mroz22 and
marekrjpolak
as code owners
| blockhash, | ||
| lastValidBlockHeight, |
There was a problem hiding this comment.
Shouldn't these values be based of the blockhash included in the transaction? That's why I originally didn't do it because it would be harder to refactor the code to pass in the correct values.
However I suppose it's not a huge deal, as the only drawback is that the user will simply wait longer for the transaction to fail, but still.
There was a problem hiding this comment.
I think it could be like that because it's working and also some thread on Solana StackOverflow is saying the same
There was a problem hiding this comment.
Ok. If pending transactions ever are implemented then I'd suggest to fix this part as well to include the blockchash from the transaction and its corresponding lastValidBlockHeight. As I mentioned in the previous comment - the only drawback of the solution in this PR should be longer than necessary wait times for transaction failure.
There was a problem hiding this comment.
I think the difference in blockhashes might be, that the txn will be considered expired later (due to this blockhash being younger than the one in the txn). But success / failure should return normally
* fix(blockchain-link): confirm solana transactions * fix(suite): wait on solana transaction confirmation in review (cherry picked from commit 3cb139f)
Description
Followup on #11033 because I don't have permissions to push to Vaccumlabs fork.
This PR improves both parts of the old PR but still it's not a final solution for handling transaction status on Solana. It would be better to handle it with pending state in transaction list and more or less make it the same UX as on other chains but let's do it later
Screenshots: