bitcoin-dev-project/sim-ln

Feature: Optional name for activity descriptions

Opened this issue · 11 comments

Add an optional "name" field on activity descriptions that can be used as a logging prefix so that it's easy to relate logs to their corresponding activity description.

Hi @carlaKC, I'll like to work on this if it's still available.

Sure! Since this is a user-facing issue, please post a brief design proposal on how you'd like to update the simulation file:

  • Changes we'd make to the json to add to the label for actvities
  • What we're going to do when we have no labels (use index in json?)
  • How we'll handle validation: do we allow some activities with labels and some without?

Great! Thank you. I'll come up with a design proposal and share with you here.

hello everyone, this is great, but can I come up with a design proposal to work on this? I want to contribute.

hello everyone, this is great, but can I come up with a design proposal to work on this? I want to contribute.

I'm working on the proposal already. Had some blockers setting up project locally earlier but fixed that already.

ohh!, its alright!

Let me look out for another issue to contribute to.

Sure! Since this is a user-facing issue, please post a brief design proposal on how you'd like to update the simulation file:

Thank you. I'm sorry I didn't come back to this immediately. I had to look at the test cases and made some draft changes to understand the moving parts first.

  • Changes we'd make to the json to add to the label for actvities

We'll add a new field to the activity description. I'm thinking we can label it activity_name or just name. Then update the following:

  • ActivityParser struct
  • ActivityDefinition struct
  • validated_activities
  • Test cases and lastly,
  • Use the activity "name" on logging.

Let me know if I'm missing something.

  • What we're going to do when we have no labels (use index in json?)

We can use the source and destination node aliases together as the label if the aliases were provided in json and not the node ID. Otherwise we use the index. For example, Alice to Bob activity can be "Alice->Bob" as the label, or "Activity 0" (where "0" is the index of the activity) as the label if the node ID was used.

  • How we'll handle validation: do we allow some activities with labels and some without?

I'm not sure what the best approach is. I'm thinking we can allow activities with or without labels, but if an activity doesn't have a label, then we can make up a label from their alias or activity index in json.

Let me know what your thoughts are. @carlaKC

We can use the source and destination node aliases together as the label if the aliases were provided in json and not the node ID. Otherwise we use the index. For example, Alice to Bob activity can be "Alice->Bob" as the label, or "Activity 0" (where "0" is the index of the activity) as the label if the node ID was used.

I don't think that we should have multiple ways of doing this depending on what's there, I think it'll be confusing. Would be happy to just use the indexes to start with. We already log the "Alice->Bob" info as part of the description, so I think that way we won't be duplicating anything.

I'm thinking we can allow activities with or without labels, but if an activity doesn't have a label, then we can make up a label from their alias or activity index in json.

SGTM!

Feel free to go ahead and open up a PR, please do open it up early/in draft for a concept ACK so that we can get an early idea of what it'll look like!

We can use the source and destination node aliases together as the label if the aliases were provided in json and not the node ID. Otherwise we use the index. For example, Alice to Bob activity can be "Alice->Bob" as the label, or "Activity 0" (where "0" is the index of the activity) as the label if the node ID was used.

I don't think that we should have multiple ways of doing this depending on what's there, I think it'll be confusing. Would be happy to just use the indexes to start with. We already log the "Alice->Bob" info as part of the description, so I think that way we won't be duplicating anything.

SGTM! I'll update to use indexes.

I'm thinking we can allow activities with or without labels, but if an activity doesn't have a label, then we can make up a label from their alias or activity index in json.

SGTM!

Thank you.

Feel free to go ahead and open up a PR, please do open it up early/in draft for a concept ACK so that we can get an early idea of what it'll look like!

Sure! Thank you so much for your response. I'll go ahead to complete this and open a PR immediately for review.

Hi @carlaKC , I opened a PR draft here. Let me know what your thoughts are when you get to take a look at it.