-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Exclude reverted events in the new events APIs #11770
Comments
Here's the problem I see with it:
In that case, I think I'd want to know about the historical reverts because there's a good chance that some of the non-revert events I have before my disconnect were reverted in the time before re-connecting and I'll have lost state. The alternative workflow to this is to not use historical at all on But then why offer historical on |
So, what we want to do is return reverts for to the events you've already observed. Unfortunately, the current APIs will never return useful reverts. given
We want reverts for blocks B, C, and D. But we will only get reverts for blocks H, I, and J. I could resubscribe at epoch 2 after getting the chain path between my last head and the current head, but that's racy. I think the only option given the current API is to call At a minimum, I think we should stop pre-filling reverted events in |
But I think a better API would be to only return reverted historical events if we specify a start tipset (both in get and subscribe). |
Right, so if you supply a tipset key then we can at least walk back to the common ancestor of current chain and give you what you need. I'm fine with that approach, because at least then I could get better guarantees of re-subscribing where I left off and getting the things I expect. |
I'm going to suggest an alternative proposal: drop the entire concept of "reverted" events (from our custom APIs) and instead tell the user when the chain itself has reverted. Either do this by :
|
That might help, because the more I try and tackle this problem as a consume of events, the more of a nightmare state management becomes, to the point where I'm considering just holding back for 900 epochs and only consuming events from there and filtering out reverted events. |
That's probably not going to work for most applications. Unfortunately, we don't have F3 yet. Although one option is to implement a finality calculator and then add an option where the user can specify that they only want finalized events. That won't make reverts impossible, but all we really need is to make reverts extremely unlikely the next 6 months. |
@rvagg Do you have a status update on this. Do you think this is worth pursuing? |
@rjan90 yes, but as outlined above in #11770 (comment), which is a change to the API - "give me events since this tipset" is what we really want to provide, but it's not a small change so will need to sit in the priority queue somewhere. |
// ChainGetPath returns a set of revert/apply operations needed to get from
// one tipset to another, for example:
// ```
// to
// ^
// from tAA
// ^ ^
// tBA tAB
// ^---*--^
// ^
// tRR
// ```
// Would return `[revert(tBA), apply(tAB), apply(tAA)]`
ChainGetPath(ctx context.Context, from types.TipSetKey, to types.TipSetKey) ([]*HeadChange, error) //perm:read this is what we would use for this API change |
We need to tell the user about reverted events when they are subscribed to events, but we don't need to tell them about historically reverted events. In the future, we may add the ability to subscribe starting at some tip set. In that case, we will want to return all events reverted between the start tip set and the first tip set with new events, but that's a future problem.
We should probably also make the reverted field optional. I'm not sure, but it would be nice to not include it in events returned from get events, only from subscriptions.
The text was updated successfully, but these errors were encountered: