substrate-api-client
substrate-api-client copied to clipboard
scs
accept retracted XtStatus
Retracted tx status means that it had been included in a block which happens to be orphaned on a non-finalized fork. We saw this happening more frequently recently and actually its not a problem in the usual flow because finality will still happen eventually as the nodes inject retracted into tx pool again automatically.
I'd suggest to accept retracted as a non-error
It may still be worth a warning in the logs
example commits (no PR because based on old release): https://github.com/brenzi/substrate-api-client/commit/480823e6a82236c4d6eb3a488a56f4bce2c4be2c https://github.com/brenzi/substrate-api-client/commit/d01bc07fe0c8a8db1451d0d5c8bf82493b274d80
@brenzi I will have a look at it. Just to clarify: The two commits from your comment would basically implement this? Or am I missing somthing?
Yes they would. Happy to submit a PR, but I first wanted to raise the discussion to see if you agree and would merge rhis
I also gave it some more thought. In general I find the current separation into expected and unexpected status quite arbitrary. Whether they are expected or not depends on the use case and I feel that by making this separation we make a decision that actually the user should take.
In polkadot there is a is_final method: https://github.com/paritytech/polkadot-sdk/blob/74a42cebc1a9fd4e4a7713d5e41caba77a0fa172/substrate/client/transaction-pool/api/src/lib.rs#L161
Maybe we should also use this in our methods. You can wait for some status but once we receive a final status we stop waiting. What do you think?
Ok, I will try to come up with a PR. Still need to do some thinking on how to do this. This change could lead to some very subtle breaks of backward compatibility for clients.
When we change to is_final instead of the current expected one, we shouldn't forget about the future status. So it probably makes sense to do https://github.com/scs/substrate-api-client/issues/295 in the same swoop.
