core: Fix duplicate VID when multiple offchain triggers fire in the same block

[hudsonhrh]

Each iteration of the offchain trigger loop in handle_offchain_triggers creates a fresh BlockState with vid_seq reset to RESERVED_VIDS (100). When two or more offchain triggers (e.g. file/ipfs data sources) fire in the same block and write to the same entity table, they produce identical VIDs, causing a unique constraint violation.

This threads vid_seq from the onchain EntityCache through the offchain trigger loop so each trigger continues the sequence where the previous one left off. Also adds unit tests demonstrating both the collision and the fix.

There are tests for illustrative purposes showing the bug and the fix

Fixes #6335

[hudsonhrh]

@lutter fixed the styling issue with 8ab10ca
thanks for triggering the workflow run

[lutter]

@lutter fixed the styling issue with 8ab10ca thanks for triggering the workflow run

No Problem. One thing I am not entirely sure about is that now everywhere an EntityCache is created, we need to remember to preserve the vid_seq. I would like to have something more automatic that is harder to screw up. Maybe we introduce an EntityCacheBuilder or some such and have as_modifications return that. The builder would then be responsible for preserving state from one cache to the next. But I haven't fully wrapped my head around how the cache and its data move through the code.

[hudsonhrh]

@lutter fixed the styling issue with 8ab10ca thanks for triggering the workflow run

No Problem. One thing I am not entirely sure about is that now everywhere an EntityCache is created, we need to remember to preserve the vid_seq. I would like to have something more automatic that is harder to screw up. Maybe we introduce an EntityCacheBuilder or some such and have as_modifications return that. The builder would then be responsible for preserving state from one cache to the next. But I haven't fully wrapped my head around how the cache and its data move through the code.

Good call. I looked into it a bit more and the blast radius is actually pretty narrow. onchain triggers share a single EntityCache passed by value, so vid_seq accumulates naturally. It's only the offchain trigger loop that needs manual threading since each trigger gets an isolated cache.

the manual read-before-consume / write-after-create pattern is definitely fragile. I think the simplest fix is adding vid_seq to ModificationsAndCache that struct already carries the LFU cache forward, so vid_seq should travel with it. then you can’t forget it because it’s part of the return value you’re already destructuring. No new abstractions needed just using existing patterns.

how does that approach sound to you i could go ahead and implement it if you think it sounds good

[lutter]

Reading the code in more detail, I think this is fine as-is

[lutter]

Can you rebase this on top of latest master? I'll merge it then

[hudsonhrh]

Can you rebase this on top of latest master? I'll merge it then

should be good now

[lutter]

Thanks for the fix!