Skip to content
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

Replace ephemeral tables with offchain tables #1123

Closed
Tracked by #760
alvrs opened this issue Jul 10, 2023 · 9 comments · Fixed by #1558
Closed
Tracked by #760

Replace ephemeral tables with offchain tables #1123

alvrs opened this issue Jul 10, 2023 · 9 comments · Fixed by #1558
Assignees

Comments

@alvrs
Copy link
Member

alvrs commented Jul 10, 2023

  • Ephemeral tables are essentially just events that take advantage of the same network stack as tables. We should rename them to events to make their use case more clear.

Todos:

  • separate from tables in config
  • StoreEphemeralRecord -> StoreEvent
  • MyEvent.emit instead of MyEvent.emitEphemeral
  • Still use existing encoding/packing
@alvrs alvrs converted this from a draft issue Jul 10, 2023
@alvrs alvrs added this to the Contracts stable milestone Jul 10, 2023
@alvrs alvrs mentioned this issue Jul 10, 2023
17 tasks
@holic
Copy link
Member

holic commented Jul 17, 2023

There's a few use cases to think about:

  • structured tables that aren't stored on chain (for gas reasons) but still should be accessible in indexer/client as if they're regular tables
  • events that emit data to trigger off-chain interactions, like animations
  • ways to associate store changes with a given system call

@holic holic changed the title Change "ephemeral tables" to "events" Decide on approaches for ephemeral tables, events, etc. Aug 24, 2023
@alvrs
Copy link
Member Author

alvrs commented Aug 31, 2023

A couple thoughts:

  • We need to decide whether we see ephemeral tables as "events" or "offchain tables".
  • The usecase of "offchain tables" is definitely valid, in which case I think we should reduce the differences between ephemeral tables and allow all table methods (setRecord, setField, pushToField, popFromField, updateInField, deleteRecord), but just not write to storage.
  • This would include some refactoring of Store code to avoid duplicate code for both the methods writing to storage and those that just emit events.
    • Ideally emitting events should be an internal function that is used by both setRecord and setRecordEphemeral (and all other functions, naming tbd).
    • Alternatively we could have a single function that takes an additional function ephemeral and conditionally executes the storage part of the function.
  • We should make sure it's not possible to emit an ephemeral event for a non-ephemeral table, because it would lead to offchain state to be different from onchain state (assuming indexers don't distinguish between ephemeral and regular tables by default)
    • This could be achieved by checking a flag in the Tables table when calling an ephemeral method to make sure they're registered as ephemeral. This would increase the cost for ephemeral table functions by around 2k (for the additional sload). No check is required in the functions for regular tables (meaning it would be possible to store data on-chain for an ephemeral table, but it's fine because there is no assumption of onchain state matching offchain state for ephemeral tables anyway)
  • offchain tables could also be used for some version of custom events (except custom event signatures), but the current version of ephemeral tables is not a full fledged offchain table (bc it's missing things like dynamic data modifications and deleting records)

@dk1a
Copy link
Contributor

dk1a commented Aug 31, 2023

I agree that offchain tables is a superset of ephemerals/events in terms of usefulness, the only downside is it's more complicated

We should make sure it's not possible to emit an ephemeral event for a non-ephemeral table, because it would lead to offchain state to be different from onchain state

I don't think we need to protect the user here, it's already easy to corrupt onchain data if you circumvent codegened libs and intentionally incorrectly call Store directly

@alvrs
Copy link
Member Author

alvrs commented Sep 1, 2023

it's already easy to corrupt onchain data if you circumvent codegened libs and intentionally incorrectly call Store directly

Can you elaborate on this / give an example for how to corrupt onchain data via external Store methods? For root systems it's of course possible to emit whatever event to make the onchain state diverge from the indexer state, and this is fine because only the root owner (usually the initial deployer of the World) can register root systems. The more important piece is that it should not be possible to make the onchain state diverge from the indexer state when writing data via the external Store methods, and in my mind this is currently true: whatever bytes data is provided to the methods is written to storage and emitted in events, and the indexer updates its state in the same way as the onchain state was updated. Every bytes value is "valid" in the sense that it can be interpreted as some type, and the interpretation is the same onchain and on the indexer. (Whether the value is semantically correct is a different question, but at least the value should be the same onchain and the indexer.)

@alvrs
Copy link
Member Author

alvrs commented Sep 1, 2023

If we wanted to support all table methods (including setField, pushToField, etc) for ephemeral tables, we would either have to add a set of function signatures for each (e.g. emitSetField, emitPushToField) to all relevant interfaces (StoreCore, Store, StoreSwitch, World), or add a bool ephemeral parameter to all functions. The former would bloat the interfaces a lot and add a lot of bytecode (maybe to the point where the World would no longer fit the contract size limit anymore since it's already close), and the latter feels kinda clunky and would increase the gas cost for all methods.

I feel like the cleanest solution that would still fit most use cases is to only support setRecord and deleteRecord for ephemeral tables. Emitting events for these is very simple and cheap, and use cases like arrays can still be solved by changing the consumer's data model (e.g. adding an index key instead of using a dynamic field).

These ephemeral tables would mostly solve the offchain table use case. For the separate use case of "custom events" we could add a separate solution: an optional root module that allows users to register custom events (each event type can be registered once, is bound to a namespace and can only be emitted by systems in that namespace). This would even allow for things like token interfaces to be implemented directly on the World contract, rather than of the workaround of an interface proxy (note that there could only be one of these interfaces per World since each event can only be registered once).

@dk1a
Copy link
Contributor

dk1a commented Sep 1, 2023

The more important piece is that it should not be possible to make the onchain state diverge from the indexer state when writing data via the external Store methods, and in my mind this is currently true

true, I suppose I don't see the divergence as a fundamentally different problem - if the contract deployer wants to scam the user, they can. And devs who use generated libs wouldn't stumble onto this problem in the first place. But for devs who do complicated stuff with Store directly this may cause a problem

an optional root module that allows users to register custom events

I think a root system can already just emit events that will be emitted by World because they're delegatecalled

@alvrs
Copy link
Member Author

alvrs commented Sep 1, 2023

if the contract deployer wants to scam the user, they can. And devs who use generated libs wouldn't stumble onto this problem in the first place. But for devs who do complicated stuff with Store directly this may cause a problem

I think the main thing we should avoid is the possibility for diverging state on vanilla Worlds without root systems (= it shouldn't be possible to have diverging state by using the permissionless external Store methods)

I think a root system can already just emit events that will be emitted by World because they're delegatecalled

I should clarify - the proposed root module adds the ability for anyone (not just the root namespace owner) to register a custom event, which can then be emitted by systems in the namespace for which the event was registered (not just root systems) by calling a method on the World.

@alvrs
Copy link
Member Author

alvrs commented Sep 1, 2023

Specific proposed changes to ephemeral tables:

  • Change the name to "offchain table" ("ephemeral" implies the state is not stored anywhere, but with this change offchain tables would be synced by indexers by default just like regular tables)
  • StoreCore
    • change emitEphemeralRecord to emitSetRecord
    • add emitDeleteRecord
    • add a flag to the Tables table to store whether a table is onchain or offchain (could be called offchainOnly or something else)
    • In emitSetRecord and emitDeleteRecord revert if the table isn't an offchain table
    • Remove StoreSetEphemeral and instead use the same StoreSetRecord and StoreDeleteRecord events as for regular tables
  • tablegen
    • change the config flag from ephemeral to whatever flag name we decide on (eg offchainOnly)
    • generate a set and deleteRecord method (just like regular tables but without the methods for fields)

@holic
Copy link
Member

holic commented Sep 1, 2023

I like the naming suggestions!

I feel like in terms of indexing, it might be worth having the separate events (or a boolean on the events) so that we don't have to look up the table registration/definition to determine if it's an offchain only table. Although the sync stack currently combines these/treats them as a single event, that was just me being lazy. I think there are decent use cases for knowing when a table is offchain only. cc @Kooshaba @yonadaaa

@alvrs alvrs changed the title Decide on approaches for ephemeral tables, events, etc. Replace ephemeral tables with offchain tables Sep 16, 2023
@alvrs alvrs moved this from Todo to Implementation in MUD v2 stable release (deprecated) Sep 16, 2023
@alvrs alvrs self-assigned this Sep 18, 2023
@alvrs alvrs linked a pull request Sep 20, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
3 participants