Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[FEAT] - Only enable ledger events if asked to #1389

Open
jasagredo opened this issue Feb 11, 2025 · 3 comments
Open

[FEAT] - Only enable ledger events if asked to #1389

jasagredo opened this issue Feb 11, 2025 · 3 comments
Assignees

Comments

@jasagredo
Copy link
Contributor

Describe the feature you'd like

At the moment we enable ledger events unconditionally (search for EPReturn). We then seem to only use them for tracing or testing.

It probably would make sense to make this dependent on some configuration flag so that ledger operations would be even cheaper.

@amesgen
Copy link
Member

amesgen commented Feb 11, 2025

We then seem to only use them for tracing or testing.

Do you have an example where we use them for tracing? (The InspectLedger stuff doesn't rely on Ledger events.)

It probably would make sense to make this dependent on some configuration flag so that ledger operations would be even cheaper.

There is some discussion on this in this thread on Slack.

In particular, @lehins said there that laziness means that we currently don't pay any additional cost (though this ticket still makes sense):

@amesgen: Just out of interest: In Consensus, we set asoEvents = STS.EPReturn here; however we don't ever look at them, so maybe lazy evaluation saves us?

@lehins: Yeah, laziness saves from expensive computation there. But I'd suggest disabling the events anyways

@jasagredo
Copy link
Contributor Author

I don't have an example, and indeed I was confused by the InspectLedger stuff. Good catch.

I'm however reading the thread and also this open PR #402 and I wonder if we would like to take this approach further:

  • Make ApplySTSOpts configurable
  • Allow for events handler to be passed into consensus

That way we could disable events and if they are enabled we could connect a handler to be used by consumers (node?)

@jasagredo
Copy link
Contributor Author

I did some work on #1390, still as a WIP

@jasagredo jasagredo self-assigned this Feb 11, 2025
@jasagredo jasagredo moved this to 🏗 In progress in Consensus Team Backlog Feb 11, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
Status: 🏗 In progress
Development

No branches or pull requests

2 participants