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

Remove ApplyBlobHooks & TxHooks from the module-system #434

Open
bkolad opened this issue Jun 27, 2023 · 0 comments
Open

Remove ApplyBlobHooks & TxHooks from the module-system #434

bkolad opened this issue Jun 27, 2023 · 0 comments
Assignees

Comments

@bkolad
Copy link
Member

bkolad commented Jun 27, 2023

Is your feature request related to a problem? Please describe.
The current design of the hook system (refer to issue #320) introduces complications. The main problem lies in the fact that the hooks cannot return arbitrary types. This limitation prevents hook composition and severely restricts the overall approach.

Describe the solution you'd like
Instead of mandating that every module implements the ApplyBlobHooks & TxHooks traits, the Runtime will take charge of implementing the RuntimeCapabilities defined in the following way:

trait RuntimeCapabilities {
    type Context: Context;

    fn get_account(
        &self,
        pub_key: &<Self::Context as Spec>::PublicKey,
        working_set: &mut WorkingSet<<Self::Context as Spec>::Storage>,
    ) -> anyhow::Result<<Self::Context as Spec>::Address>;

    fn increment_nonce(
        &self,
        pub_key: &<Self::Context as Spec>::PublicKey,
        working_set: &mut WorkingSet<<Self::Context as Spec>::Storage>,
    ) -> anyhow::Result<()>;

    fn is_sequencer_allowed(&self, blob: &mut impl BlobTransactionTrait) -> bool;
}

The AppTemplate will receive the concrete implementation of RuntimeCapabilities. This enables the runtime hooks to have a specific purpose, allowing us to utilize their return types in the logic of the AppTemplate.

Solution as of 2023-07-19

Hooks get stricter

  1. post blob/tx hooks do not return result. they can only modify state and it is commited afterwards
  2. state is always commited after pre hooks
  3. pre hooks can change control flow and skip body (blob or tx processing)

Default-STF-Hooks drawio

Context builder traits introduced

For functionality when STF needs some data from modules, 2 ContexBuilders are introduced:

TxContextBuilder

It is very similar to current pre_dispatch_hook, but instead of returning Address it returns whole context.

pub trait TxContextBuilder {
    type Context: Context;

    fn build_tx_context(
        &self,
        tx: &Transaction<Self::Context>,
        working_set: &mut WorkingSet<<Self::Context as Spec>::Storage>,
    ) -> Self::Context;
}
  • It does not error, it always succeeds or panics

  • It is called before pre/tx hooks

  • state is not commited

  • ? DoS Vector: spam state with non existing accounts? Currently we create account if it is not in database.

BlobContextBuilder

Currently not needed, so won't be implemented

SlotBuilder

Module can implement this trait, with following interface:

TBD

@citizen-stig citizen-stig self-assigned this Jul 21, 2023
@citizen-stig citizen-stig changed the title RemoveApplyBlobHooks & TxHooks from the module-system Remove ApplyBlobHooks & TxHooks from the module-system Jul 21, 2023
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

2 participants