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

Investigate what happens when events sender is not configured #1896

Closed
mversic opened this issue Feb 9, 2022 · 4 comments
Closed

Investigate what happens when events sender is not configured #1896

mversic opened this issue Feb 9, 2022 · 4 comments
Assignees
Labels
Bug Something isn't working good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@mversic
Copy link
Contributor

mversic commented Feb 9, 2022

This piece of code in core/src/wsv.rs should be investigated:

    fn produce_events(&self, events: impl IntoIterator<Item = DataEvent>) {
        let events = events.into_iter().map(Event::from);
        let events_sender = if let Some(sender) = &self.events_sender {
            sender
        } else {
            return warn!("wsv does not equip an events sender");
        };
        for event in events {
            drop(events_sender.send(event))
        }
    }

I can't say why this compiles, maybe it panics inside the macro. Probably an error should be returned or events just dropped

@mversic mversic added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Feb 9, 2022
@appetrosyan appetrosyan added the Bug Something isn't working label Feb 11, 2022
@appetrosyan appetrosyan added the good first issue Good for newcomers label Apr 19, 2022
@Erigara
Copy link
Contributor

Erigara commented Jun 1, 2022

As far as i can see warn! doesn't panic inside, when i try to call produce_event on default WorldStateView where events_sender is None (see following snippet) and read through macro expanded version of produce_event function i don't notice anything suspicious.

It seems like event is just discarded if events_sender is None.

  #[test]
  fn produce_event_none_events_sender() {
      let block = ValidBlock::new_dummy().commit();
      let wsv = WorldStateView::<World>::default();
      let event = wsv.create_time_event(&block).unwrap();
      wsv.produce_event(event);
  }

I can't say why this compiles

@mversic Could you please explain why it shouldn't compile.

@mversic
Copy link
Contributor Author

mversic commented Jun 1, 2022

ok, I was confused by the return statement. It's all good here

@Erigara
Copy link
Contributor

Erigara commented Jun 1, 2022

Maybe to avoid confusion we could refactor produce_event to something like this?

  /// Send [`Event`]s to known subscribers.
  fn produce_event(&self, event: impl Into<Event>) {
      if let Some(events_sender) = &self.events_sender {
          drop(events_sender.send(event.into()));
      } else {
          warn!("wsv does not equip an events sender");
      }
  }

@mversic
Copy link
Contributor Author

mversic commented Jun 1, 2022

Yeah, if you wish

Erigara added a commit to Erigara/iroha that referenced this issue Jun 1, 2022
…ation

Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
Erigara added a commit to Erigara/iroha that referenced this issue Jun 1, 2022
…ation

Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
Erigara added a commit that referenced this issue Jun 2, 2022
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
@Erigara Erigara closed this as completed Jun 2, 2022
BAStos525 pushed a commit to BAStos525/soramitsu-iroha that referenced this issue Jul 8, 2022
…ation

Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
Signed-off-by: BAStos525 <jungle.vas@yandex.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

No branches or pull requests

3 participants