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

Trimming events to contain essential attributes only #5901

Open
3 tasks
colin-axner opened this issue Feb 26, 2024 · 3 comments
Open
3 tasks

Trimming events to contain essential attributes only #5901

colin-axner opened this issue Feb 26, 2024 · 3 comments
Labels
needs discussion Issues that need discussion before they can be worked on type: performance Performance or storage optimizations
Milestone

Comments

@colin-axner
Copy link
Contributor

Summary

As per request from Osmosis, sometimes we emit events which aren't essential or provide duplicate information. This issue is a placeholder to discuss potentially removing event attributes which do not provide useful benefits and cost more in usage.

02-client event trimming

The client type should be unnecessary as it is contained in the clientID. Maybe it is used in some workflow for querying events?

We should be able to remove the client type in all 02-client queries

04-channel event trimming

On recv packet, we emit all the information needed to reconstruct the packet, but the packet is provided as an argument to the MsgRecvPacket, see events. All that should really be necessary is the packet sequence and port/channel

The same is true for write acknowledgement, see events

In acknowledge packet events, I think we could just emit the packet id information (sequence, source port, source channel)

For timeout packet, I think we could do packet ID info (sequence, source port, source channel) + timeout info


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner added the type: performance Performance or storage optimizations label Feb 26, 2024
@colin-axner colin-axner added the needs discussion Issues that need discussion before they can be worked on label Feb 26, 2024
@colin-axner
Copy link
Contributor Author

A very minimal recv packet event could look like this:

// emitRecvPacketEvent emits a receive packet event. It will be emitted both the first time a packet
// is received for a certain sequence and for all duplicate receives.
func emitRecvPacketEvent(ctx sdk.Context, packet types.Packet, channel types.Channel) {
    ctx.EventManager().EmitEvents(sdk.Events{
        sdk.NewEvent(
            types.EventTypeRecvPacket,
            sdk.NewAttribute(types.AttributeKeySequence, fmt.Sprintf("%d", packet.GetSequence())),
            sdk.NewAttribute(types.AttributeKeyDstPort, packet.GetDestPort()),
            sdk.NewAttribute(types.AttributeKeyDstChannel, packet.GetDestChannel()),
        ),
        sdk.NewEvent(
            sdk.EventTypeMessage,
            sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
        ),
    })
}

and for write acknowledgement:

// emitWriteAcknowledgementEvent emits an event that the relayer can query for
func emitWriteAcknowledgementEvent(ctx sdk.Context, packet types.Packet, channel types.Channel, acknowledgement []byte) {
    ctx.EventManager().EmitEvents(sdk.Events{
        sdk.NewEvent(
            types.EventTypeWriteAck,
            sdk.NewAttribute(types.AttributeKeySequence, fmt.Sprintf("%d", packet.GetSequence())),
            sdk.NewAttribute(types.AttributeKeyDstPort, packet.GetDestPort()),
            sdk.NewAttribute(types.AttributeKeyDstChannel, packet.GetDestChannel()),
            sdk.NewAttribute(types.AttributeKeyAck, string(acknowledgement)), //nolint:staticcheck // DEPRECATED
            sdk.NewAttribute(types.AttributeKeyAckHex, hex.EncodeToString(acknowledgement)),
        ),
        sdk.NewEvent(
            sdk.EventTypeMessage,
            sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
        ),
    })
}

and for acknowledge packet:

// emitAcknowledgePacketEvent emits an acknowledge packet event. It will be emitted both the first time
// a packet is acknowledged for a certain sequence and for all duplicate acknowledgements.
func emitAcknowledgePacketEvent(ctx sdk.Context, packet types.Packet, channel types.Channel) {
    ctx.EventManager().EmitEvents(sdk.Events{
        sdk.NewEvent(
            types.EventTypeAcknowledgePacket,
            sdk.NewAttribute(types.AttributeKeySequence, fmt.Sprintf("%d", packet.GetSequence())),
            sdk.NewAttribute(types.AttributeKeySrcPort, packet.GetSourcePort()),
            sdk.NewAttribute(types.AttributeKeySrcChannel, packet.GetSourceChannel()),
        ),
        sdk.NewEvent(
            sdk.EventTypeMessage,
            sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
        ),
    })
}

and for timeout packet

// emitTimeoutPacketEvent emits a timeout packet event. It will be emitted both the first time a packet
// is timed out for a certain sequence and for all duplicate timeouts.
func emitTimeoutPacketEvent(ctx sdk.Context, packet types.Packet, channel types.Channel) {
    ctx.EventManager().EmitEvents(sdk.Events{
        sdk.NewEvent(
            types.EventTypeTimeoutPacket,
            sdk.NewAttribute(types.AttributeKeyTimeoutHeight, packet.GetTimeoutHeight().String()),
            sdk.NewAttribute(types.AttributeKeyTimeoutTimestamp, fmt.Sprintf("%d", packet.GetTimeoutTimestamp())),
            sdk.NewAttribute(types.AttributeKeySequence, fmt.Sprintf("%d", packet.GetSequence())),
            sdk.NewAttribute(types.AttributeKeySrcPort, packet.GetSourcePort()),
            sdk.NewAttribute(types.AttributeKeySrcChannel, packet.GetSourceChannel()),
        ),
        sdk.NewEvent(
            sdk.EventTypeMessage,
            sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
        ),
    })
}

@colin-axner
Copy link
Contributor Author

The most effective starting point is to try to remove the redundant packet data emissions in the recv packet flow (currently emitted 4 times)

@colin-axner
Copy link
Contributor Author

requires informalsystems/hermes#4078

@colin-axner colin-axner removed their assignment Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Issues that need discussion before they can be worked on type: performance Performance or storage optimizations
Projects
Status: On hold ❌
Development

No branches or pull requests

2 participants