Skip to content

Commit

Permalink
feat(node_framework): Only store each wiring layer once (#1468)
Browse files Browse the repository at this point in the history
## What ❔

Changes the logic of the framework so that it's OK to call `add_layer`
with the same layer multiple times: the corresponding layer will only be
stored once.

## Why ❔

It may be handy if the same layer (e.g. think gas adjuster) is a
prerequisite for multiple other layers: we can simply bundle them
together without having to worry whether any other layer adds it.

It will be useful for components-based system.

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [ ] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [ ] Tests for the changes have been added / updated.
- [ ] Documentation comments have been added / updated.
- [ ] Code has been formatted via `zk fmt` and `zk lint`.
- [ ] Spellcheck has been run via `zk spellcheck`.
- [ ] Linkcheck has been run via `zk linkcheck`.
  • Loading branch information
popzxc committed Mar 21, 2024
1 parent ac4a744 commit 4a393dc
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 6 deletions.
13 changes: 12 additions & 1 deletion core/node/node_framework/src/service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,19 @@ impl ZkStackServiceBuilder {
/// Adds a wiring layer.
/// During the [`run`](ZkStackService::run) call the service will invoke
/// `wire` method of every layer in the order they were added.
///
/// This method may be invoked multiple times with the same layer type, but the
/// layer will only be stored once (meaning that 2nd attempt to add the same layer will be ignored).
/// This may be useful if the same layer is a prerequisite for multiple other layers: it is safe
/// to add it multiple times, and it will only be wired once.
pub fn add_layer<T: WiringLayer>(&mut self, layer: T) -> &mut Self {
self.layers.push(Box::new(layer));
if !self
.layers
.iter()
.any(|existing_layer| existing_layer.layer_name() == layer.layer_name())
{
self.layers.push(Box::new(layer));
}
self
}

Expand Down
34 changes: 29 additions & 5 deletions core/node/node_framework/src/service/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,33 +24,57 @@ fn test_new_with_nested_runtime() {
}

#[derive(Debug)]
struct DefaultLayer;
struct DefaultLayer {
name: &'static str,
}

#[async_trait::async_trait]
impl WiringLayer for DefaultLayer {
fn layer_name(&self) -> &'static str {
"default_layer"
self.name
}

async fn wire(self: Box<Self>, mut _node: ServiceContext<'_>) -> Result<(), WiringError> {
Ok(())
}
}

// `ZkStack` Service's `add_layer()` method has to add multiple layers into `self.layers`.
// `add_layer` should add multiple layers.
#[test]
fn test_add_layer() {
let mut zk_stack_service = ZkStackServiceBuilder::new();
zk_stack_service
.add_layer(DefaultLayer)
.add_layer(DefaultLayer);
.add_layer(DefaultLayer {
name: "first_layer",
})
.add_layer(DefaultLayer {
name: "second_layer",
});
let actual_layers_len = zk_stack_service.layers.len();
assert_eq!(
2, actual_layers_len,
"Incorrect number of layers in the service"
);
}

// `add_layer` should ignore already added layers.
#[test]
fn test_layers_are_unique() {
let mut zk_stack_service = ZkStackServiceBuilder::new();
zk_stack_service
.add_layer(DefaultLayer {
name: "default_layer",
})
.add_layer(DefaultLayer {
name: "default_layer",
});
let actual_layers_len = zk_stack_service.layers.len();
assert_eq!(
1, actual_layers_len,
"Incorrect number of layers in the service"
);
}

// `ZkStack` Service's `run()` method has to return error if there is no tasks added.
#[test]
fn test_run_with_no_tasks() {
Expand Down

0 comments on commit 4a393dc

Please sign in to comment.