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

Use a struct type for client identifiers #3173

Closed
3 tasks
colin-axner opened this issue Feb 20, 2023 · 2 comments
Closed
3 tasks

Use a struct type for client identifiers #3173

colin-axner opened this issue Feb 20, 2023 · 2 comments
Labels
icebox Issues that we will not address for the time being type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.

Comments

@colin-axner
Copy link
Contributor

Summary

Use a struct to manage client identifiers.

Create the type:

type ClientID struct {
    Type       string
    Identifier uint64 // or some other name, Nonce, Sequence?
}

Support the following functions:

func (ClientID) String() string
func (ClientID) IsValid() bool
func (ClientID) IsLocalhost() bool
func ParseClientIdentifier(clientID string) ClientID

Problem Definition

It can be difficult to validate the correctness of our identifier creation/validation since we pass around a string and reference non struct functions defined here.

Proposal

Leave implementation as is for now

I've opened this issue for discussion/idea documentation, but as it could be a major API breaking change, I think it's best to leave this issue as food for thought until we are confident we could make the change without any disruption. It may be useful to use the ClientID for a limited section of the codebase, only change internal APIs.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner added the icebox Issues that we will not address for the time being label Feb 20, 2023
@colin-axner
Copy link
Contributor Author

Ah, one issue I already see is that the client identifier format cannot be enforced on counterparty chain values, so this type could only be used to reference ClientID's on the executing chain. Counterparty Client ID types would need to remain being a string. Though this could help clarify the difference in expected usage between the two (counterparty client identifiers should be treated as opaque strings)

@crodriguezvega crodriguezvega added the type: refactor Architecture, code or CI improvements that may or may not tackle technical debt. label Jul 12, 2024
@colin-axner
Copy link
Contributor Author

I don't think this issue will ever be picked up, so going to close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
icebox Issues that we will not address for the time being type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Projects
None yet
Development

No branches or pull requests

2 participants