Cut pulled GlobalSnapshotEvents, if necessary

[ryle-goehausen]

These events on the input to the cut function are not ordered themselves so there is no way to correctly pick the ones that will pass because ordering by fee and hash doesn't abide to the discrete ordering of the events

Since we're decoding the currency snapshot and hence have an ordinal, there is a validation ordering that should be imposed. We should rely on the chain data ordering and validation to determine this, and otherwise allow RBF policies.

[TheMMaciek]

Since we're decoding the currency snapshot and hence have an ordinal, there is a validation ordering that should be imposed. We should rely on the chain data ordering and validation to determine this, and otherwise allow RBF policies.

FYI not all metagraph snapshots need to be the CurrencySnapshot so we can't always rely that we will be able to order by ordinal. StateChannelSnapshotBinary has an order based on the lastSnapshotHash so we can figure out the order of these events - we have functions doing that as a part of artifact creation. We also accept blocks with a recursive function that effectively builds an order in which blocks can be accepted.

[ryle-goehausen]

FYI not all metagraph snapshots need to be the CurrencySnapshot so we can't always rely that we will be able to order by ordinal. StateChannelSnapshotBinary has an order based on the lastSnapshotHash so we can figure out the order of these events - we have functions doing that as a part of artifact creation. We also accept blocks with a recursive function that effectively builds an order in which blocks can be accepted.

I think so long as we're imposing a validator on lastSnapshotHash and enforcing a synchronous acceptance, it should be okay here (rejecting the batch event case for now, or we can discuss in meeting.) And if we decode a currency snapshot, we can enforce an ordinal validation logic, that way any future events will be rejected as invalid until the active GS changes. But again let's discuss sync-validation versus batching strategies in meeting.

[TheMMaciek]

Given that we only want a rough ordering, the PR seems mostly fine. I guess the orderings make sense - correct me if I'm wrong but least significant events are at the head of the list right? Only few stylistic comments added.

[sdavidp]

I think it looks ok given the assumptions for this functionality. One last thing I could suggest is to merge the cut and removeEvents functions into one cut function. You could move the size check to happen first in the recursion and only if the size exceeds the limit cut one element and start another loop. Also to avoid unnecessary sorting you would need to make scEventsSorted and dagEventsSorted to be lazy vals. This could make this functionality a bit simpler, but it's only a suggestion.

EDIT: I think I'm wrong about the sorted events being lazy vals, they will either way be evaluated immediately even if the size doesn't exceed the limit - so it won't give us anything and the sorting will always happen which is not what we want. Also we will always build the EventWithFee collections even if it's not necessary in the end. So I guess the current implementation is probably optimal.

My intention was to structure the calls so that we don't build the intermediate data structures or sort unless we know there's going to be some cutting.