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

Modular systems / system to system calls #319

Closed
alvrs opened this issue Jan 5, 2023 · 7 comments
Closed

Modular systems / system to system calls #319

alvrs opened this issue Jan 5, 2023 · 7 comments
Labels
discussion A discussion (without clear action/outcome)

Comments

@alvrs
Copy link
Member

alvrs commented Jan 5, 2023

Problem description

Systems encapsulate logic. It would be nice if it was possible to use systems as modular building blocks to create higher level systems, but when a system is called from another system, any access control logic related to msg.sender fails, because the calling system is the new msg.sender.

A common workaround is to encapsulate logic in libraries instead, use those libraries as building blocks and use systems only as entry points to the application. However, this comes with two issues:

  1. It is hard to keep track of which components the libraries need write access to. Write access control to components happens on the level of systems, not libraries, therefore every system using a library needs to be granted write access to the components the library writes to.
  2. This workaround works for development, when the deployer of the components and systems is the same, so systems can be granted write access to any component. But it is not possible for third party developers to use the logic encapsulated in libraries as building blocks for their own higher-level systems (because they're lacking write access to the required components).

Example of a situation where it would be nice to use systems as building blocks, but we run into the issue of msg.sender being different:

  • A MoveSystem has access to a PositionComponent and implements the rules of movement in the world (eg. the entity to be moved needs to be owned by the system caller (msg.sender), and the target position needs to be adjacent to the current position)
  • Imagine a third party developer wants to extend the World by adding a PathSystem, which batches calls to MoveSystem to move multiple steps in one transaction. In an ideal world PathSystem could just call MoveSystem multiple times with valid steps, thus respecting the existing rules and adding a higher level system.
  • But the call from PathSystem to MoveSystem fails, because msg.sender is PathSystem and not the owner of the entity to be moved.

Approaches

1. access control via tx.origin

2. Subsystems

  • Proposed in Add subsystem #268, see Modular systems / system to system calls #319 (comment)
  • Downsides to this approach
    • Development issues are solved, but it's still not possible for third party developers to use systems as building blocks to build higher level systems. (Which is intentional in this approach, because subsystems are supposed to implement partial logic that might assume permission checks to be done by the parent system).

3. Global entry point

  • Another rough idea that was floating around was to add a global entry point to the World contract.
  • The entry point function could store the caller in a temporary variable on the World, before calling the requested systems, and finally resetting the caller variable to some placeholder value.
  • Systems could use this global caller variable for access control instead of msg.sender, thereby allowing systems to call other systems while preserving the initial caller.
  • Downsides to this approach
    • This approach feels similar to using the generally frowned upon tx.origin for access control, with the exception of supporting smart contracts wallets, and maybe less phishing potential because the entry point call has to be explicit (instead of the possible use of fallback functions with tx.origin).
    • This approach would add a small gas overhead to every entry point call (around 5800 if the global caller variable is never set to 0 but another non-zero placeholder value).
    • With the ability to upgrade systems it might be possible to trick users into calling a malicious system, which then calls another system with full permissions of the user.
@alvrs alvrs added the discussion A discussion (without clear action/outcome) label Jan 5, 2023
@dk1a
Copy link
Contributor

dk1a commented Jan 5, 2023

Back when I made #268 I myself wasn't sure subsystems were useful. Now I actually have a bunch of examples, so here's an updated pitch.

System to system calls aren't common. Small projects wouldn't need them at all.
Libraries are great, use them whenever possible.

Open this deploy.json, scroll down to CycleCombatSystem.
It uses some components and CombatSubsystem, which in turn uses some components, DurationSubsystem and EffectSubsystem
All these subsystems are also used by other (sub)systems, not just CycleCombatSystem.
(See #303 for details on EffectSubsystem and DurationSubsystem and callbacks and them using each other)

What's a subsystem?
Just a system that inherits OwnableWritable, whereas normally systems inherit only Ownable. Subsystems are systems.

Why do I need OwnableWritable?
I don't want users to call subsystems. They're basically stateful libraries. Components already have authorizeWriter, lets reuse it.

Why not just use Systems then?

  1. You'd have to add OwnableWritable (authorizeWriter etc) to the required systems yourself. Inconvenient boilerplate.
  2. writeAccess in deploy.json. Subsystems as primitves can be built into cli. You don't wanna modify LibDeploy.ejs manually every time.
  3. Subsystem is a different name. It makes it clear at glance what's external and what's internal.

Why not just use libraries?

  1. Often I do.
  2. Can't, contracts have a size limit.
  3. Callbacks (see feat(std-contracts): add SystemCallbackComponent #303) are impossible with libs.
  4. I don't even know how many components CycleCombatSystem uses if you unroll the subsystems. Manually managing huge writeAccesses is very inconvenient.
  5. Libraries don't have writeAccess. Subsystems do. You can easily see which components/subsystems a subsystem writes to. You have to read the whole library to know what it writes to.
  6. Statefulness can be convenient. I don't need to pass world or components to a subsystem.

Subsystem examples:
ERC721BaseSubsystem (a special case, this one can't be a library not for any of the above reasons, but because it has to be stateful due to events)
ERC1155BaseSubsystem (same thing)
ScopedDurationSubsystem
EffectSubsystem
CombatSubsystem (this is a perfect example of "Can't, contracts have a size limit". It also has 2 dedicated libs, to keep it less bloated)
EquipmentSubsystem (btw this one isn't aware of items or characters or anything. It's a really abstract utility to equip stuff to slots)
RandomMapSubsystem (this is an example of a subsystem I think may be better of as a library instead, I'm not sure yet)

@alvrs
Copy link
Member Author

alvrs commented Jan 5, 2023

@dk1a thanks for the updated pitch / explanation of subsystems and how they compare to regular systems and libraries.
I just updated the issue description with a more detailed problem description and potential solutions.

We can definitely agree on the usefulness of "stateful libraries" or calling systems from other systems. In my opinion the subsystem approach perfectly solves the "first party" issues (adding the possibility for stateful libraries for a developer's own systems), but I'd like to think about whether we can come up with a solution that would give the same benefits to third party developers (using existing systems as modules to create more complex higher level systems).

Curious to hear your thoughts on this.

@dk1a
Copy link
Contributor

dk1a commented Jan 5, 2023

@alvrs Interesting, I've never thought of that side of the problem in relation to subsystems.
I haven't really gotten there yet with my game (I just use msg.sender atm), but what I've been planning is to have more of a bottom-up approach. So just don't use simple permissions.

Lets say you have a character entity, and a MoveSystem.
Then you have an ApprovalComponent with mapping(characterEntity=>approvedAddress=>approvedSystem)
(The implementation details are hazy, the mapping can be done e.g. by hashing 2 of 3 or something like that)
Then you have some convenient utility to set and check generic permissions (it can check msg.sender too).
Then if you wanna use PathSystem, call something like approve(getAddressById(PathSystemID), MoveSystemID, charEntity)
Maybe add some utility to recs or std-client that can largely automate this.

approvedAddress instead of approvedSystemId maybe avoids upgradeability phishing, but not really (it could call an upgradeable system, so is it even worth it?)

my details may be off, haven't actually written any code for this

@dk1a
Copy link
Contributor

dk1a commented Jan 5, 2023

The main point of that whole approval thing is, it's ok if you get phished. You character will just move incorrectly for a while. The attacker doesn't get full permission to do whatever

@dk1a
Copy link
Contributor

dk1a commented Jan 5, 2023

@alvrs
Copy link
Member Author

alvrs commented Jan 6, 2023

Very interesting idea! It got me thinking about how we could use a general approval pattern to solve this issue (modular systems) as well as the session wallet issue (to move on from pure burner wallets). Created a proposal in #327, lmk what you think!

@alvrs alvrs unpinned this issue Jan 6, 2023
@holic holic added this to the Contracts stable milestone Jul 10, 2023
@alvrs
Copy link
Member Author

alvrs commented Jul 10, 2023

Closing this since it's pre v2 (we basically implemented subsystems, and the global entry point is part of #1124)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion A discussion (without clear action/outcome)
Projects
None yet
Development

No branches or pull requests

3 participants