Actor event entries do not retain order when passing through event index

[rvagg]

The event index doesn't care about order of "entries" that come with events, as per schema:

lotus/chain/events/filter/index.go

Lines 51 to 58 in a1264c8

	`CREATE TABLE IF NOT EXISTS event_entry (
	event_id INTEGER,
	indexed INTEGER NOT NULL,
	flags BLOB NOT NULL,
	key TEXT NOT NULL,
	codec INTEGER,
	value BLOB NOT NULL
	)`,

Unfortunately, order now matters. As per FIP-0083 both sector-activated and sector-updated rely on piece-cid and piece-size pairings to come out in order such that you can join them up to get the "PieceInfo". When we rely on sqlite table ordering, we get a shuffling of the array of events which loses pairings.

The result is that, when using GetActorEventsRaw (and SubscribeActorEventsRaw for historical events) we get something like this (from calibnet msg bafy2bzacec2e6fiovhcjrprjgi76rhoqhr6bt7ag7dsejng5sefuksyvmvpwu):

[
  {
    "Flags": 1,
    "Key": "piece-size",
    "Codec": 81,
    "Value": "GwAAAAgAAAAA"
  },
  {
    "Flags": 3,
    "Key": "$type",
    "Codec": 81,
    "Value": "bnNlY3Rvci11cGRhdGVk"
  },
  {
    "Flags": 3,
    "Key": "piece-cid",
    "Codec": 81,
    "Value": "2CpYKAABgeIDkiAg8pxbycLUKlq7v8IZyXva5vs6aSxy9/mz5t2ygXvZjAw="
  },
  {
    "Flags": 3,
    "Key": "sector",
    "Codec": 81,
    "Value": "GScT"
  },
  {
    "Flags": 3,
    "Key": "unsealed-cid",
    "Codec": 81,
    "Value": "2CpYKAABgeIDkiAg8pxbycLUKlq7v8IZyXva5vs6aSxy9/mz5t2ygXvZjAw="
  }
]

When loading the same from the from the AMT with ChainGetEvents, we see the original order:

[
  {
    "Flags": 3,
    "Key": "$type",
    "Codec": 81,
    "Value": "bnNlY3Rvci11cGRhdGVk"
  },
  {
    "Flags": 3,
    "Key": "sector",
    "Codec": 81,
    "Value": "GScT"
  },
  {
    "Flags": 3,
    "Key": "unsealed-cid",
    "Codec": 81,
    "Value": "2CpYKAABgeIDkiAg8pxbycLUKlq7v8IZyXva5vs6aSxy9/mz5t2ygXvZjAw="
  },
  {
    "Flags": 3,
    "Key": "piece-cid",
    "Codec": 81,
    "Value": "2CpYKAABgeIDkiAg8pxbycLUKlq7v8IZyXva5vs6aSxy9/mz5t2ygXvZjAw="
  },
  {
    "Flags": 1,
    "Key": "piece-size",
    "Codec": 81,
    "Value": "GwAAAAgAAAAA"
  }
]

We're using a INSERT OR IGNORE INTO when putting event_entry values in but we only have an index (on key), not a UNIQUE so I believe that means we're still going to get the duplicate keys, but I'd like someone else to sanity check that assumption. (Aside: we also don't have an index on event_id which is the only thing we're actually selecting (joining) on, which seems odd, maybe we should).

[rvagg]

Some reasons this hasn't been picked up earlier:

1. We don't have much (any?) real data with multiple pieces in DDO sectors, even on calibnet (I haven't seen one yet), so this piece-cid + piece-size thing hasn't made much of a dent.
2. We use require.ElementsMatch in itests that look at event entry specifics (there are a few cases where we check the arrays specifically, but ignore order!)
3. SubscribeActorEventsRaw shouldn't have this problem when looking at live events before they round-trip through the db.

Workaround goes something like this:

• Don't assume $type comes first, even though that would be nice, to figure out an event type look through all entries to find $type.
• When dealing with a sector-activated or sector-updated event, you should go through ChainGetEvents. But for that you need the AMT root, which you have to get from StateSearchMsg to look for the message CID that you get with the event, that call gives you EventsRoot which you can then pass to ChainGetEvents and get the events decoded straight out of the block store if it hasn't been pruned by GC.

[rvagg]

Some additional tidbits:

• Aggregate piece-size should still be fine - if you only care about total non-CC size in a sector then ✅
• The claim and allocation* events from verifreg have piece-cid and piece-size (singular) so you can match up verified pieces from sector-activated and sector-updated (which you would probably do anyway if you were interested in verified onboarding) ✅
• Given the above, if you're matching, you can get accurate aggregate verified and aggregate unverified numbers for each sector ✅
• You can't use this data to do anything specific with unverified pieces if you care about their size+CID pair, I can't think of much utility of these pairings for unverified though, so maybe this is less important? ❌

Regarding a fix, I think what we need to do is:

• Add a order int field to the event_entry table and increment from 0 for each event_id and then order by that when we select out.
• Migration has some options:
  • Just migrate and set everything existing to be order 0 or some other arbitrary order number
  • During migration, if we encounter a builtin actor event (Codec=81 is a solid tell), go fishing around in the message receipts for the original and if we have it (we may not) then put in the correct ordering.

[aarshkshah1992]

@rvagg Raised #11829 for this.

See discussion on Slack. We don't need to do a migration as we're okay with calibnet having jumbled events before the epoch where we'll patch it with this. There's only like 10 or so deals on calibnet currently.

Please can you test this out with the flow which led you to discover this bug in the first place and maybe modify the actors events itest in my PR to test for this ?

[Stebalien]

Related discussion: #11830

[Stebalien]

I'm actually kind of surprised by this issue. Given the lack of a primary key, shouldn't SQL be giving us an automatic auto-incrementing primary key? See https://www.sqlite.org/withoutrowid.html

I wonder if we can just rename this row ID to an actual primary key and then order by it. That should give us event fields in insertion order. Kind of implicit, but it should work.

[aarshkshah1992]

@Stebalien I think we still need specify that "automatic auto-incrementing primary key" in the DDL for the "event_entry" table (just like we do it for the event table). I don't think we just get it without asking for it. But then it's really the same as what we've done in #11829 (we explicitly insert an "index" column).

[Stebalien]

By default, every row in SQLite has a special column, usually called the "rowid", that uniquely identifies that row within the table. However if the phrase "WITHOUT ROWID" is added to the end of a CREATE TABLE statement, then the special "rowid" column is omitted. There are sometimes space and performance advantages to omitting the rowid.