morph: make client retry if fallback transaction was accepted#3798
morph: make client retry if fallback transaction was accepted#3798
Conversation
carpawell
requested review from
End-rey,
cthulhu-rider and
roman-khimov
as code owners
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3798 +/- ##
==========================================
+ Coverage 25.55% 25.56% +0.01%
==========================================
Files 660 660
Lines 42663 42664 +1
==========================================
+ Hits 10903 10909 +6
+ Misses 30753 30751 -2
+ Partials 1007 1004 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pkg/morph/client/static.go
Outdated
| err := invokeFunc() | ||
| if err != nil { | ||
| if errors.Is(err, neorpc.ErrMempoolCapReached) { | ||
| if errors.Is(err, neorpc.ErrMempoolCapReached) || errors.Is(err, notary.ErrFallbackAccepted) { |
There was a problem hiding this comment.
Fallback? Why anyone cares about fallback here?
There was a problem hiding this comment.
if IR cannot accept transaction, doesn't it lead to FB being accepted instead?
There was a problem hiding this comment.
But it has no problems accepting a transaction in #3739.
There was a problem hiding this comment.
hm, i got you. then i can suggest retrying every time we get a non-HALT tx state for every CallWithAlphabetWitness call (likely that will lead to many backoffs when anything is not right in vm execution, any correct panic in our contract call), or we may do a string search for insufficient amount of gas
There was a problem hiding this comment.
Out of GAS only. If execution fails for other reasons that's likely a permanent error (for example, container was locked before deletion attempt and now it can't be deleted).
There was a problem hiding this comment.
added search for insufficient amount of gas. also can be a narrower System.Storage.Get failed
If complex notary invocation flow failed, retry transaction sending the same way it is done for `ErrMempoolCapReached` problems. Contract storages (and, therefore, GAS payments) may change b/w SN's test invocations and real transaction acceptance tries, and it is normal. In particular, it fixes #3739. Signed-off-by: Pavel Karpy <[email protected]>
carpawell
force-pushed
the
feat/container-removal-retries
branch
from
9572ea8 to
a2d3310
Compare
2 participants
If complex notary invocation flow failed, retry transaction sending the same way it is done for
ErrMempoolCapReachedproblems. Contract storages (and, therefore, GAS payments) may change b/w SN's test invocations and real transaction acceptance tries, and it is normal. In particular, it fixes #3739.