search

bitcoin-dev-project / sim-ln

Payment activity generator for the lightning network
MIT License
62 stars 28 forks source link

Feature: Optional name for activity descriptions #127

Open carlaKC opened 1 year ago

carlaKC commented 1 year ago

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.

Anyitechs commented 8 months ago

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

carlaKC commented 8 months ago

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

Anyitechs commented 8 months ago

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

ikeogu commented 8 months ago

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

Anyitechs commented 8 months ago

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.

ikeogu commented 8 months ago

ohh!, its alright!

ikeogu commented 8 months ago

Let me look out for another issue to contribute to.

Anyitechs commented 8 months ago

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:

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

carlaKC commented 8 months ago

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!

Anyitechs commented 8 months ago

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.

Anyitechs commented 8 months ago

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