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

Add "Sent" event when a message is sent via execute #6851

Open
franciscoaguirre opened this issue Dec 11, 2024 · 3 comments
Open

Add "Sent" event when a message is sent via execute #6851

franciscoaguirre opened this issue Dec 11, 2024 · 3 comments
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. I5-enhancement An additional feature request. T6-XCM This PR/Issue is related to XCM.

Comments

@franciscoaguirre
Copy link
Contributor

With the extrinsics in pallet-xcm, we always emit the Sent event when a message is sent as a result of our actions:

let e = Event::Sent { origin, destination: dest, message: remote_xcm, message_id };

We're moving towards a future where we encourage the usage of pallet-xcm.execute() over highly specialized extrinsics. One missing piece for this is that when calling execute, no event is emitted when a message is sent.
We should emit the Sent event everytime execution results in sending a message:

The problem with this is that emitting events is something pallets do, not the executor, so we don't have access to it. We have access to the XcmRouter which delivers the message.
We have to give the executor access to the pallet in some way.

Two ways that I see of doing this:

  1. Adding a new configuration item, something like EventEmitter, which we can plug in and use to emit the event we want. Downside of this is it's a breaking change since we add a new associated type to the config.
  2. We provide the pallet as another generic of a particular XcmRouter, for example ParentAsUmp and then use that to emit events. With this we don't introduce a breaking change but still manage to emit events as long as our routers have this logic.
@franciscoaguirre franciscoaguirre added the T6-XCM This PR/Issue is related to XCM. label Dec 11, 2024
@acatangiu
Copy link
Contributor

Can we make the executor emit events? I believe there are multiple places in the executor where we should emit events for visibility and traceability - ideally executor would emit these directly.

Otherwise, we can go with your first suggestion and add config EventEmitter and have pallet_xcm implement and emit all types of relevant events.

@franciscoaguirre
Copy link
Contributor Author

Events are a FRAME concept. The executor uses the config for accessing these functionalities so I think we should use that mechanism for events.

A single config for emitting all events might be a good compromise. However, this would be a breaking change. We could do that for stable2503 so we pile all events we want there but we can backport option 2.

@acatangiu
Copy link
Contributor

I would go with the proper change even if it is braking and thus cannot be backported. I would prefer to avoid technical debt where we can, and this issue doesn't look urgent enough to warrant taking on temporary workarounds.

So, I ultimately support your option 1: add config EventEmitter and have pallet_xcm implement and emit all types of relevant events.

@acatangiu acatangiu added C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. I5-enhancement An additional feature request. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. labels Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. I5-enhancement An additional feature request. T6-XCM This PR/Issue is related to XCM.
Projects
Status: No status
Development

No branches or pull requests

2 participants