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

Proposal: World framework v2 #393

Closed
alvrs opened this issue Feb 10, 2023 · 8 comments
Closed

Proposal: World framework v2 #393

alvrs opened this issue Feb 10, 2023 · 8 comments

Comments

@alvrs
Copy link
Member

alvrs commented Feb 10, 2023

Table of contents

Abstract

This proposal outlines a new version of the World framework, which leverages the Store library to move away from individual component contracts to manage state and instead towards managing state in access-controlled tables (backwards compatible to components) in a central World contract. In addition it proposes a new way of interacting with systems via a central entry point on the World contract. This new architecture promises to be more gas-efficient and powerful, enabling a long list of extensions described in the last part.

Motivation

The goal of the World framework is to facilitate complex and extendable on-chain applications. If used exclusively by a first party developer it can be compared to the Diamond pattern. But the goal of the World framework is allow both first and third party developers to extend existing applications, after they’ve been deployed, to facilitate the creation of Autonomous Worlds.

Issues with the previous approach

In the first version of MUD, the World contract served as a central registry for components and systems and as a central source of update events for indexers and clients to reconstruct the component state. State and logic was separated into components and systems, with components being separate contracts (each managing its own access control) and systems being bound to a World (with the World address stored in their storage). This approach led to a couple issues / inconveniences:

  • Each system contract can only be part of a single World (unlike in the Diamond pattern where Facets can be part of different diamonds, making it possible to create a new instance of an app by redeploying a single DiamondProxy contract and linking existing Facets to it)

  • Unnecessary gas overhead for reading from / writing to components from a system

    1. System loads componentRegistry contract address from its storage (1x sload)
    2. System calls componentRegistry to retrieve the address of the component to read from / write to given its id (1x call, 1x sload)
    3. System calls component to read/write data (1x call, 1x sload / sstore)

    → 2x call, 2x sload overhead in addition to the required sload / sstore

  • Logic for account delegation or to allow for systems to call other systems (eg Proposal: General approval pattern (for modular systems and session wallets) #327) would have to be implemented in each system separately (since systems are called directly)

  • Access control managed in each component contract separately means developers have to either manually keep track of which systems need access to which component, or just give all systems access to all components, with the number of permissions to be set scaling quadratically (number of systems * number of components)

Relation to Store

In #347 / #352 we added a new core storage paradigm to allow any contract to become compatible with the MUD networking stack (while also saving on storage gas due to more efficient packing).

In this proposal we use the new Store library to move all state to the central World contract instead of individual component contracts, and extend the low level Store functionality with access control for individual storage areas to allow a World app to be extended by independent third party developers while preventing unauthorised storage access.

Design goals

With this proposal we aim to make the World framework:

  1. as gas efficient as possible (as little gas overhead compared to a “regular contract” as possible) - especially during runtime (gas efficiency during deployment / setup has a lower priority than gas efficiency for users)
  2. as extendable / general as possible to make this framework useful for a wide range of complex on-chain applications
  3. as developer friendly as possible (no boilerplate, type-safety where possible)

Basic Concepts

This section goes over the general concepts that are important for this proposal.

Separation of data and logic into tables and systems

  • All state lives in the storage of the World contract. We use the Store package to make storage as cheap as possible and automatically emit events for any state change for indexers / clients to reconstruct the application state.
  • All logic lives in modular system contracts.
    • Systems are not called directly, but via a central entry point in the World contract. They trust the msgSender value forwarded by the World contract.
    • System contracts don’t contain any state and don’t have any reference to the World they belong to, so they can be registered in different Worlds (akin to Facets in the Diamond pattern)
    • Systems read state from the World and write state to the World. The method of reading/writing state is abstracted from the system implementation in a library:
      • if the system is delegatecalled from the World, it reads/writes directly to the delegated World storage (using the Store library)
      • if the system is called from the World, it reads/writes via access controlled methods on the World contract (by calling the methods on msg.sender, which always is the World contract)
    • Systems don’t need to verify that they are called by a World contract, because they don’t contain any state and always read/write on their caller.

Routes

  • A route is a string ending in / (like /mudswap/ or /skystrife/round1/)
  • Systems and tables are registered in the World at a route and addressed via their path (like /mudswap/BalanceTable or /skystrife/round1/Movementsystem)
  • Routes can have arbitrary depth
  • Each route has an owner
  • Only the owner of a route can register tables and systems at the route
  • Anyone can register a new route by extending an existing route by one fragment. The address registering the route becomes the route’s owner.
    • Eg: Assume Alice owns /skystrife/. Only Alice can register tables or systems at /skystrife/. But Bob can register /skystrife/bobmod/ and then register tables and systems at this new route.
  • Systems registered at the root level (/) are called via delegatecall, all other systems are called via call

Access control

  • Access control for writing to tables and executing systems is based on routes.
  • If an address has access to a route, it also has access to all subroutes.
    • Eg: Assume Charlie has access to /skystrife/, then Charlie can also access /skystrife/bobmod/PositionTable
  • Systems can be public (= anyone can call them) or private (= only addresses with explicit access can call them)
  • Systems automatically get access to the base route they are registered at
    • Eg. a system registered at /skystrife/Movementsystem has access to /skystrife/ and therefore has access to all tables and systems registered under /skystrife/

Code exploration

This section includes a “pseudo-code” implementation of the framework described above. The code is close to real Solidity, but has no ambitions to compile or to be complete.

// Since systems are only called via a World contract, we exclusively use _msgSender() instead of msg.sender.
// This allows the World to manage account delegation without any specific logic required in systems.
contract System {
  // Extract the trusted msg.sender value appended to the calldata
  function _msgSender() internal pure returns (address sender) {
    assembly {
      // 96 = 256 - 20 * 8
      sender := shr(96, calldataload(sub(calldatasize(), 20)))
    }
  }
}

contract World {
	error RouteExists(string route);
	error RouteInvalid(string route);
	error RouteAccessDenied(string route);
	error SystemExists(address system);

	constructor() {
		// Create native tables
		RouteTable.registerSchema();
		RouteAccessTable.registerSchema();
		OwnerTable.registerSchema();
		SystemTable.registerSchema();
		SystemToRouteTable.registerSchema();	
		
		// Register root route and give ownership to msg.sender
		bytes32 rootRouteId = keccak256(bytes(""));
		OwnerTable.set({ key: rootRouteId, owner: msg.sender });
		RouteAccessTable.set({ key: [rootRouteId, msg.sender], access: true });
	}

	/***************************** REGISTRATION METHODS *****************************/

	/**
	 * Register a new route by extending an existing route
	 */
	function registerRoute(string calldata baseRoute, string calldata subRoute) returns (bytes32 routeId) {
		// Require subroute to end with `/` and not include any other `/`
		if(!_isTopLevelRoute(subRoute)) revert RouteInvalid(subRoute);

		// Construct the new route
		string memory route = abi.encodePacked(parentRoute, subRoute);
		bytes32 routeId = keccak256(route);

		// Require route to not exist yet
		if(RouteTable.has(routeId)) revert RouteExists(route);
		
		// Add caller as owner of the new route
		OwnerTable.set({ key: routeId, owner: msg.sender });

		// Give caller access to the route
		RouteAccessTable.set({ key: [routeId, msg.sender], access: true });
	}

	/**
	 * Register register a table with given schema at the given route
	 */
	function registerTable(string calldata baseRoute, string calldata tableRoute, Schema schema) {
		// Register table route
		bytes32 routeId = registerRoute(baseRoute, tableRoute);

		// StoreCore handles checking for existence
		StoreCore.registerSchema(routeId, schema);
	}

	/**
	 * Register a system at the given route
	 */
	function registerSystem(string calldata baseRoute, string calldata systemRoute, System system, bool openAccess) {
		// Require the system to not exist yet
		if(SystemToRoute.has(address(system))) revert SystemExists(systemAddress);

		// Require the caller to own the base route
		bytes32 baseRouteId = keccak256(bytes(baseRoute);
		if(OwnerTable.get(baseRouteId) != msg.sender) revert RouteAccessDenied(baseRoute);

		// Register system route
		bytes32 systemRouteId = registerRoute(baseRoute, systemRoute);

		// Store the system address in the system table
		SystemTable.set({ key: systemRouteId, system: systemAddress, openAccess: openAccess });

		// Store the system's route in the SystemToRoute table
		SystemToRoute.set({ key: systemAddress, route: systemRouteId });
		
		// Give the system access to its base route
		RouteAccessTable.set({ key: [baseRouteid, systemAddress], access: true });
	}

	/**
	 * Grant access to a given route
	 */
	function grantAccess(string calldata route, address grantee) {
		// Require the caller to own the route
		bytes32 routeId = keccak256(bytes(route));
		if(OwnerTable.get(routeId) != msg.sender) revert RouteAccessDenied(baseRoute);

		// Grant access to the given route
		RouteAccessTable.set({ key: [routeId, grantee], access: true });
	}
	
	// TODO method to retract access

	/***************************** STORE METHODS *****************************/

	/**
	 * Write to a table based on a parent route access right
	 * We check for access based on `accessRoute`, and write to `accessRoute`/`tableRoute`
	 * because access to a route also grants access to all sub routes
	 * NOTE: this doesn't conform with the IStore interface (which just expects a single tableId),
	 *       but we extend the interface for Worlds with route based access checks.
	 *       In addition, the world implements the IStore interface and require permission on the individual
	 *       table instead of its parent route (see below)
	 */
	function setRecord(string calldata accessRoute, string calldata tableRoute, bytes32[] calldata key, bytes calldata data) {
		// Check access control based on the `accessRoute`
		bytes32 accessRouteId = keccak256(bytes(accessRoute));
		if(!RouteAccessTable.get({ key: [accessRouteId, msg.sender] })) revert RouteAccessDenied(accessRoute);
		
		// Construct the table route id by concatenating accessRoute and subRoute
		bytes32 tableRouteId = keccak256(abi.encodePacked(accessRoute, subRoute));
		
		// Set the record
		StoreCore.setRecord(tableRouteId, key, data);
	}

	/**
	 * Write to a table based on individual access right
	 * (to conform with the IStore interface)
	 */
	function setRecord(bytes32 tableRouteId, bytes32[] calldata key, bytes calldata data) {
		// Check access based on the tableRoute
		if(!RouteAccessTable.get({ key: [tableRouteId, msg.sender] })) revert RouteAccessDenied(tableRouteId);
		
		// Set the record
		StoreCore.setRecord(tableRouteId, key, data);
	}

	// TODO: add functions for `setField` and `deleteRecord` akin to `setRecord`

	/***************************** SYSTEM CALLS *****************************/

	/**
	 * Call a system based on a parent route access right
	 * We check for access based on `accessRoute`, and execute `accessRoute`/`callRoute`
	 * because access to a route also grants access to all sub routes
	 */
	function call(string calldata accessRoute, string calldata callRoute, bytes4 funcSelector, bytes calldata args) returns (bytes memory) {
		// Check if the system is a public system and get its address
		string memory systemRoute = abi.encodePacked(accessRoute, callRoute);
		bytes32 systemRouteId = keccak256(systemRoute);
		(address systemAddress, bool openAccess) = SystemTable.get(systemRouteId);

		// If the system is not public, check for individual access
		if(!openAccess) {
			bytes32 accessRouteId = keccak256(bytes(accessRoute));
			bool access = RouteAccessTable.get([accessRoute, msg.sender]);	
			if(!access) revert RouteAccessDenied(accessRoute);
		}
		
		// Call the system and forward any return data
		return _call({
			msgSender: msg.sender,
			systemAddress: systemAddress,
			funcSelector: funcSelector,
			args: args,
			delegate: _isTopLevelRoute(systemRoute)
		});
	}

	/* Overload for the function above to check access based on the full system route instead of a parent route (better devex for public systems) */
	function call(string calldata systemRoute, bytes4 funcSelector, bytes calldata args) {
		call(systemRoute, "", funcSelector, args);
	}

	/* Internal function to call system with delegatecall/call and notify hooks, without access control */
	function _call(address msgSender, address systemAddress, bytes4 funcSelector, bytes memory args, bool delegate) internal returns (bytes memory) {
		// Call the system using `delegatecall` for root systems and `call` for others
		// and append msg.sender to the calldata
		(bool success, bytes memory data) = delegate ?
			? systemAddress.delegatecall(abi.encodeWithSelector(funcSelector, args, msgSender)) // root system
			: systemAddress.call(abi.encodeWithSelector(funcSelector, args, msgSender)); // non-root system

		// Forward returndata
		if(success) return data;
		
		// Forward error if the call failed
		revert(data);
	}
}

/**
 * Return true if the route includes exactly one `/`
 */
function _isTopLevelRoute(string calldata route) returns (bool) {
	// TODO: implement
}

/***************************** TABLE DEFINITIONS *****************************/
// (Libraries to interact with the tables are auto-generated by _Store_)

// RouteKey: [routeId]
struct RouteSchema {
	string preImage;
}

// RouteAccessKey: [routeId, caller]
struct RouteAccessSchema {
	bool access;
}

// OwnerKey: [owned]
struct OwnerSchema {
	address owner;
}

// SystemKey: [routeId]
struct SystemSchema {
	bool openAccess;
	address system;
}

// SystemToRouteKey: [systemAddress]
struct SystemToRouteSchema {
	bytes32 routeId;
}

Advanced concepts / extensions

With the basic World framework described above in place, we can add a couple of interesting extensions, described below.


Hooks for system calls

Akin to hooks for state changes in Store, we can add hooks for system calls. These hooks allow the owner of a route to register a “callback function” to be called when the system is called. This can be used for all kinds of use-cases, an example for an ERC20 implementation using this method is described below in “Interface proxy”.

Pseudocode for system call hooks:

interface ISystemHook {
	function onCallSystem(address msgSender, address systemAddress, bytes4 funcSelector, bytes memory args);
}

contract World {
	...

	/**
	 * Register a hook for a given route.
	 * The caller must own the accessRoute to register a hook at concat(accessRoute, systemRoute)
	 * The hook is called when a system registered at this hook is called
	 */
	function registerSystemHook(string calldata baseRoute, string calldata systemRoute, ISystemHook hook) {
		// Require the caller to own the access route
		bytes32 baseRouteId = keccak256(bytes(baseRoute);
		if(OwnerTable.get(baseRouteId) != _msgSender()) revert RouteAccessDenied(baseRoute);

		// Register new system hook
		bytes32 systemRouteId = keccak256(abi.encodePacked(baseRoute, systemRoute));
		SystemHookTable.push({ key: systemRouteId, hook: hook });
	}
	
	/* Internal function to call system with delegatecall/call and notify hooks, without access control */
	function _call(address msgSender, address systemAddress, bytes4 funcSelector, bytes memory args, bool delegate) internal returns (bytes memory) {
		// Notify system call hooks
		address[] memory hooks = SystemHooksTable.get(keccak256(systemRoute));
		for (uint256 i = 0; i < hooks.length; i++) {
			ISystemHook hook = ISystemHook(hooks[i]);
			hook.onCallSystem(msgSender, systemAddress, funcSelector, args);
		}
		...
	}
	...
}

Interface proxy

Assume we want to create an ERC20 implementation that lives inside our World, so we can share access to changing balances between different systems.

To conform with the ERC20 spec, we need to implement the ERC20 interface - but systems in our world are called via their route instead of the required function selector.

High level approach to solving this using an “Interface proxy” and system call hooks (see above):

  • InterfaceProxy contract implements ERC20 interface and ISystemHook interface
  • InterfaceProxy contract registers its tables on the world, as well as a InterfaceImplementation system (which has access to the tables)
  • InterfaceProxy forwards calls to its methods to the respective methods on the InterfaceImplementation via routes on the World
  • InterfaceProxy registers itself as hook for calls of the InterfaceImplementation system
    • Whenever InterfaceImplementation functions are called (via the World), InterfaceProxy is notified via its onCallsystem function and can emit the required ERC20 events
  • Access to InterfaceImplementation can either be limited to InterfaceProxy, or shared with individual other trusted systems, or be public (and implement its own access control checks)

Register function selectors for root systems

In the proposal above we have a single entry point on the World contract to call systems (call). By allowing the registration of function selectors for root level systems, we can dynamically add more entry points with alternative access control mechanisms! (This is restricted to the owner of the root route (/) of course.)

Pseudocode for function selectors for root systems:

contract World {
	...	
	function registerRootFunctionSelector(string calldata route, bytes4 funcSelector) {
		// Require the sender to be the root owner
		if(OwnerTable.get(keccak256(bytes(""))) != msg.sender) revert RouteAccessDenied(route);

		// Require the route to be a root route
		if(!_isTopLevelRoute(route)) revert RouteInvalid(route);
		
		// Require the function selector to not exist yet
		if(FunctionSelectorTable.has(funcSelector)) revert FunctionSelectorExists(funcSelector);

		// Get the system address
		address systemAddess = SystemTable.get(keccak256(bytes(route)));
		if(systemAddress == address(0)) revert RouteInvalid(route);
	
		// Register the function selector
		FunctionSelectorTable.set({ key: funcSelector, system: systemAddress });
	}
	...

	fallback() external payable {
		// Find system based on function selector
		address system = FunctionSelectorTable.get(msg.sig);
		
		// Delegate call the system (as this is a root system)
		// (snippet from https://eips.ethereum.org/EIPS/eip-2535)
		assembly {
			// copy function selector and any arguments
			calldatacopy(0, 0, calldatasize())
			// execute function call using the facet
			let result := delegatecall(gas(), system, 0, calldatasize(), 0, 0)
			// get any return value
			returndatacopy(0, 0, returndatasize())
			// return any return value or error back to the caller
			switch result
				case 0 {revert(0, returndatasize())}
				default {return (0, returndatasize())}
		}
	}
}

Subsystems

In #268 we explored the concept of “subsystems” as a way to share “stateful” low level logic between different systems. With the new World framework, we can revisit this concept. Now subsystems can just be private systems (that only grant access to other systems at the same base route). To call a system B as subsystem from system A, system A can just delegatecall system B directly. If system A was delegatecalled (as a root system), then all storage and context is forwarded to system B. If system A was called (as a regular system), then the msg.sender stays the same (= the World address), so system B can write to its msg.sender (= the World) as usual.


Generic account delegation / approval pattern

In addition to the single call entry point to call systems described above, we can add an alternative callFrom entry point to implement an account delegation / approval pattern similar to the ERC20 pattern of approvals and transferFrom. This was already discussed in #327, but unlike in the linked discussion, we only have to add the required logic once, in the World contract, not in every system contract separately. Systems don’t have to care about this, in fact even systems developed before the callFrom entry point was added would be compatible by default.


Modules

Modules can be a way to abstract the installation of a set of systems and corresponding tables into a World. Modules themselves are verifiable contracts, and could pave the way to an “on-chain library of World extensions”.

For simplicity the “registration methods” in the World pseudocode above were placed into the World contract directly. If instead they would live in a Registrationsystem that is registered when the World is created, we could dogfood the callFrom entry point described above to let Modules register new tables and systems on behalf of other addresses.

Pseudocode of two Module variants:

// First, the user has to approve the module.
// Then, the user calls the Module's install function, which registers tables and systems
// on the World and does any required plumbing

// This variant of a TokenModule deploys a new TransferSystem for every time it is installed.
// This means it can be installed multiple times at different routes.
contract TokenModule {
	Schema balanceTableSchema;

	function install(IWorld world, string calldata baseRoute) {
		// Register BalanceTable
		world.callFrom(msg.sender, "/registerTable", abi.encode(RegisterTableProps({
			parentRoute: baseRoute,
			tableRoute: "/balanceTable",
			schema: balanceTableSchema
		})));

		// Deploy new transfer system
		TransferSystem transferSystem = new TransferSystem(baseRoute);

		// Register TransferSystem
		world.callFrom(msg.sender, "/registerSystem", abi.encode(RegisterSystemProps({
			parentRoute: baseRoute,
			systemRoute: "/transfer",
			systemAddress: address(transferSystem)
		})));
	}
}

// This variant of a Token module doesn't require a new TransferSystem to be deployed,
// but the module can only be installed once per World.
contract TokenModule {
	string moduleRoute = "/bestTokenModule";
	Schema balanceTableSchema;
	address transferSystem; // deployed once

	function install(IWorld world, string calldata baseRoute) {
		// Register module route
		world.registerRoute(moduleRoute);

		// Register table at known route
		world.registerTable(moduleRoute, "/balanceTable", balanceTableSchema);

		// Grant transferSystem access to the known route
		world.grantAccess(moduleRoute, transferSystem);

		// Register transferSystem in the World
		world.callFrom(msg.sender, "/registerSystem", abi.encode(RegisterSystemProps({
			parentRoute: baseRoute,
			systemRoute: "/transfer",
			systemAddress: transferSystem
		})));
	}
}

Multicall

In addition to the World’s call entrypoint described above, we could add a multicall entrypoint to allow multiple systems to be executed in a single atomic transaction. (This could be extended to pipe the output of a system to the next system.)

Pseudocode of simple multicall entrypoint:

struct ISystemCall {
	string accessRoute,
	string callRoute,
	bytes4 funcSelector,
	bytes args
}

contract World {
	...
	function multicall(SystemCall[] calls) {
		for(uint256 i; i < calls.length; i++) {
			call(calls[i]);
		}
	}
}

Acknowledgements

@alvrs alvrs pinned this issue Feb 10, 2023
@dk1a
Copy link
Contributor

dk1a commented Feb 11, 2023

Everything looks great!

// Require subroute to end with / and not include any other /

you could also require no / and then append it


Might be useful for _isTopLevelRoute:
dk1a/solidity-stringutils@dfb9916 (memIsAscii can also be adapted to efficiently check for just /)
And for multicall:
https://github.com/mds1/multicall/blob/master/src/Multicall3.sol

@holic
Copy link
Member

holic commented Feb 16, 2023

I am worried that the filesystem-like routing will be a constant pain point for us/our users because of the trailing or no-trailing slash. I'm a seasoned engineer and still run into this issue constantly with bash, etc.

An alternative that comes to mind is an e.g. colon-separated namespace and wildcards:

  • /some/path = some:path
  • /some/ = some:*
  • / = *

Another thing that comes to mind with strings is escaping, if you e.g. want to use the separator in your path name (whether it's / or *). Do we just disallow usage of the separator character? Would an array of strings (each string being a path segment), rather than a single string (full path), help with this?

In terms of naming, I don't think of "routes" as access control. I wonder if a term like "namespace" is more intuitive (certainly is to me).

@alvrs
Copy link
Member Author

alvrs commented Feb 16, 2023

I am worried that the filesystem-like routing will be a constant pain point for us/our users because of the trailing or no-trailing slash. I'm a seasoned engineer and still run into this issue constantly with bash, etc.

An alternative that comes to mind is an e.g. colon-separated namespace and wildcards:

  • /some/path = some:path
  • /some/ = some:*
  • / = *

Independent from the separation character we should enforce that a route always starts with the separator and never ends with it to remove the uncertainty. I see how existing associations with / and routes might lead to confusion with trailing slashes and how using an alternative separation character like : might remove some of those existing associations. Although existing associations are also kind of the reason why I like / - it's familiar to people from both URLs / REST API routes, as well as from file systems. Do you think if we never use trailing slash anywhere and throw an explicit error if a trailing slash is used we might prevent some of the pain while still benefiting from a feeling of familiarity?

Wildcards are hard because of the way we check that a subRoute is in fact a sub route of the access route. Without wildcards we can hash the access route to check access, and then concatenate the two and hash the result to get the route id to access (-> sub route is guaranteed to be a sub route of access route because the id is obtained by concatenation).

Another thing that comes to mind with strings is escaping, if you e.g. want to use the separator in your path name (whether it's / or *). Do we just disallow usage of the separator character? Would an array of strings (each string being a path segment), rather than a single string (full path), help with this?

An array of strings would certainly be explicit here, but I'm a little wary of that option because of the gas overhead (32 bytes length overhead for every string + 32 bytes for the array) and the bad devex of defining dynamic length arrays in solidity (first define the array, then set every value separately). I also assume a higher gas cost for processing the array input compared to directly hashing the string to get it's routeId.

When registering a new route, we should definitely check that the new route doesn't include any /. Therefore extending routes is only possible one level at a time and we don't end up with routes without owner / no access rights.

In terms of naming, I don't think of "routes" as access control. I wonder if a term like "namespace" is more intuitive (certainly is to me).

I like "namespace" for referring to it in the context of access control! While for accessing a specific resource at some path I personally find "route" more intuitive. In my head its like "a route defines a namespace", you refer to a resource (table, system) with a route and give access based on namespaces, but it all uses the same underlying concept of a route.

@dk1a
Copy link
Contributor

dk1a commented Feb 16, 2023

When registering a new route, we should definitely check that the new route doesn't include any /. Therefore extending routes is only possible one level at a time and we don't end up with routes without owner / no access rights.

Should routes include special characters at all? (except for the separators) I'd limit them to alphanumeric (and maybe _)

@norswap
Copy link

norswap commented Feb 20, 2023

This is really good stuff! A couple of questions/remarks:

  1. registerTable and registerSystem call registerRoute, but shouldn't the subroute for those not end with a /? (i.e. only routes that are not systems or tables should end with /)

  2. Probably a good idea to have a name for "non-system non-table route". I think you used "access route"? That's good, probably a definition section would be useful for when this becomes documentation.

  3. (Checking my understanding) Do table access control mean that table implementation need to be implementation with basic access control to check if the caller is an "owner" (which for use with the World framework would always be set to the World contract)?

  4. Would it be good to have a scheme for permission inheritance? Right now, to have different permissions between two system, they need to live under different access routes. Could it make sense to define some routes such that only its owner can create sub-routes, and addresses with acces to the parent access route get access to all sub-access-routes automatically (but the reverse is not true)?

  5. In general, is there a point to nesting access routes currently? i.e. there seems to be no relationship at all between /myapp/ and /myapp/bob, which might as well been located at /myapp-bob/ instead.

  6. Read/write access: currently access to systems seem binary, but it's likely that some systems are strongly tied to some tables, and compute derived data from them. Under the current scheme, it's necessary to split the read and write part of the logic related to these tables, with the write part probably needing to call into the read part. Could we solve this by separating out read & write access? Or is it too much hassle and better to force writing two separate contracts?

  7. Might be worth to have hooks that execute before a system call and hooks that execute after. We could store them together to avoid the extra read in case where one or both of these two lists is empty.

  8. I love the callFrom and module stuff <3

@alvrs
Copy link
Member Author

alvrs commented Mar 2, 2023

Thanks for your comments, really good thoughts!

  1. registerTable and registerSystem call registerRoute, but shouldn't the subroute for those not end with a /? (i.e. only routes that are not systems or tables should end with /)

To avoid any confusion with trailing slashes we decided to never have any trailing slashes anywhere, so neither systems, nor tables, nor accessRoute, nor subRoute can have a trailing slash - instead all routes have a leading slash (except for the root route, which is "" (empty string)). The code in the proposal above is not 100% up to date anymore, you can find the latest state here: https://github.com/latticexyz/mud/blob/main/packages/world/src/World.sol.

  1. Probably a good idea to have a name for "non-system non-table route". I think you used "access route"? That's good, probably a definition section would be useful for when this becomes documentation.

Yeah I agree, the naming is not super clear right now. We're thinking of calling the resources at routes "files" (see #456), and making FOLDER an explicit file type to avoid some of that confusion. route could also become path to make the association with a file system a bit tighter (seems more fitting than the REST route association).

  1. (Checking my understanding) Do table access control mean that table implementation need to be implementation with basic access control to check if the caller is an "owner" (which for use with the World framework would always be set to the World contract)?

So all tables are just storage addresses within the Store (which in this case is the World), with StoreCore managing the storage locations and emitting the appropriate events. In the context of the World framework, access control is implemented once in the World contract itself - in the current prototype you can write to a table if you have access to any parent route of the table or the table's direct route. The "owner" of a table is the address that registered the table. The owner of the table can grant (and revoke) write access to the table to any other address. Note that this access permission is implemented as "owner of the route" and "access to the route", not a special case for tables. (So also an owner of the table's parent route can grant and revoke access).

  1. Would it be good to have a scheme for permission inheritance? Right now, to have different permissions between two system, they need to live under different access routes. Could it make sense to define some routes such that only its owner can create sub-routes, and addresses with acces to the parent access route get access to all sub-access-routes automatically (but the reverse is not true)?

Part of what you describe is already possible: Addresses with access to a parent route automatically have access to all sub-routes, but the reverse is not true. This is the case because the subRoute can be a nested route (this might not be super clear from the proposal), so if Alice has access to /alice, and Bob creates a sub route /alice/bob/xyz, then Alice can access the sub route by providing /alice as accessRoute and /bob/xyz as subRoute. The opposite is not true: Bob doesn't automatically get access to /alice just because he registered a sub route, but he has access to the sub route, so he can only provide /alice/bob as access route, and eg couldn't access /alice/charlie. Because registering a sub route doesn't grant any access to the parent route, we don't have to limit who can register a sub route.

  1. In general, is there a point to nesting access routes currently? i.e. there seems to be no relationship at all between /myapp/ and /myapp/bob, which might as well been located at /myapp-bob/ instead.

Related to the point above, this might have been unclear from the proposal - with nested routes permission inheritance is possible, anyone with access to /myapp can also access /myapp/bob by providing /myapp as access route. This makes it easy for a developer to give all their systems access to all their tables, but if needed also to organise tables/systems in deeper nested groups to be able to give more fine grained access (instead of just "all" or "individual table").

  1. Read/write access: currently access to systems seem binary, but it's likely that some systems are strongly tied to some tables, and compute derived data from them. Under the current scheme, it's necessary to split the read and write part of the logic related to these tables, with the write part probably needing to call into the read part. Could we solve this by separating out read & write access? Or is it too much hassle and better to force writing two separate contracts?

Read access is not limited, anyone can ready any data from any table. For the second part I think you're describing something like "Subsystems" - systems that encapsulate some logic for other systems to share, but are not intended to be called directly. For this use case we're currently thinking it might make sense to make the sub systems private (= openAccess flag set to false, which means the systems can't be called directly by the World) and then just delegatecall the subsystems from the public main systems. Since they are delegatecalled, they can access all tables the main system can access, but not any other tables.

I might be misunderstanding your point though, let me know if that's the case.

  1. Might be worth to have hooks that execute before a system call and hooks that execute after. We could store them together to avoid the extra read in case where one or both of these two lists is empty.

Totally agree, good point!

  1. I love the callFrom and module stuff <3

❤️

@norswap
Copy link

norswap commented Mar 9, 2023

So all tables are just storage addresses within the Store (which in this case is the World), with StoreCore managing the storage locations and emitting the appropriate events. In the context of the World framework, access control is implemented once in the World contract itself - in the current prototype you can write to a table if you have access to any parent route of the table or the table's direct route. The "owner" of a table is the address that registered the table. The owner of the table can grant (and revoke) write access to the table to any other address. Note that this access permission is implemented as "owner of the route" and "access to the route", not a special case for tables. (So also an owner of the table's parent route can grant and revoke access).

What prevents a system from using the libraries for a table that it does not have access to?

7. Would it be good to have a scheme for permission inheritance? Right now, to have different permissions between two system, they need to live under different access routes. Could it make sense to define some routes such that only its owner can create sub-routes, and addresses with acces to the parent access route get access to all sub-access-routes automatically (but the reverse is not true)?

Part of what you describe is already possible: Addresses with access to a parent route automatically have access to all sub-routes, but the reverse is not true. This is the case because the subRoute can be a nested route (this might not be super clear from the proposal), so if Alice has access to /alice, and Bob creates a sub route /alice/bob/xyz, then Alice can access the sub route by providing /alice as accessRoute and /bob/xyz as subRoute. The opposite is not true: Bob doesn't automatically get access to /alice just because he registered a sub route, but he has access to the sub route, so he can only provide /alice/bob as access route, and eg couldn't access /alice/charlie. Because registering a sub route doesn't grant any access to the parent route, we don't have to limit who can register a sub route.

Yeah, seems I was confused here. I wonder if what I wanted to suggest wasn't the opposite: give the sub-routes access to the parent routes (hence why only the creator could create sub-routes). Feels like often you add new features that need to call into the "lower-level" features, which would maybe more naturally be parent routes (as they were created first).

11. Read/write access: currently access to systems seem binary, but it's likely that some systems are strongly tied to some tables, and compute derived data from them. Under the current scheme, it's necessary to split the read and write part of the logic related to these tables, with the write part probably needing to call into the read part. Could we solve this by separating out read & write access? Or is it too much hassle and better to force writing two separate contracts?

Read access is not limited, anyone can ready any data from any table. For the second part I think you're describing something like "Subsystems" - systems that encapsulate some logic for other systems to share, but are not intended to be called directly. For this use case we're currently thinking it might make sense to make the sub systems private (= openAccess flag set to false, which means the systems can't be called directly by the World) and then just delegatecall the subsystems from the public main systems. Since they are delegatecalled, they can access all tables the main system can access, but not any other tables.

I might be misunderstanding your point though, let me know if that's the case.

What I meant is that it would be good to group all logic related to a given table into a given system. This includes data derivation logic (which should be public, ~ view solidity functions) and write logic (which should only be available to people who are allowed to call the system).

Access to the table is not enough, as you want to reuse the data derivation logic.

Sure, subsystems will solve it (pretty similar to what I said of separate read (data derivation) / write logic), but then you have to write two contracts dealing with the same table, which might not be tons of code each.

Adding a read/write distinction on subsystems might be too complex / too much overhead to bother though. It was just an idea.

@alvrs
Copy link
Member Author

alvrs commented Jul 10, 2023

Closing this since we started implementing it in v2 and discussion on individual parts is happening in dedicated issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants