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

Routing ISM #5

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Routing ISM #5

wants to merge 3 commits into from

Conversation

asaj
Copy link
Contributor

@asaj asaj commented Dec 21, 2022

No description provided.

@asaj asaj marked this pull request as ready for review December 21, 2022 18:09

When relayers detect a `RoutingIsm`, metadata should be formatted in accordance with the `ISM` type being routed to.

`RoutingIsms` should avoid conditioning on frequently-changing state, as this may result in a relayer formatting metadata for the wrong `ISM`.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to call this out here? This feels like an implementation detail of a relayer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless you have a workaround to suggest, this seems like good practice, and worth calling out.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's just me but I think RoutingISMs should do whatever they want and relayers should account for their fetching for metadata being out of date by retrying


`RoutingIsms` should avoid conditioning on frequently-changing state, as this may result in a relayer formatting metadata for the wrong `ISM`.

### **Rationale**
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume these sections are to be filled out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh not really sure what there is to put, I can't think of any design decisions that merit discussion

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If none apply maybe just a small N/A or something like that would be nice to assert completeness

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants