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

Add helper for checking if a connection exists and is open #3466

Closed
3 tasks
DimitrisJim opened this issue Apr 17, 2023 · 2 comments
Closed
3 tasks

Add helper for checking if a connection exists and is open #3466

DimitrisJim opened this issue Apr 17, 2023 · 2 comments
Assignees
Labels

Comments

@DimitrisJim
Copy link
Contributor

DimitrisJim commented Apr 17, 2023

Summary

There's many instances of code that requires a connection to exist and be open:

  1. During AcknowledgePacket

  2. During RecvPacket

  3. During ChanOpenTry

  4. During ChanOpenAck

  5. During ChanOpenConfirm

  6. During ChanCloseInit

  7. During ChanCloseConfirm

Additionally, it seems it will also be required in validateProposedUpgradeFields as added at some point in #3451 where it was initially conceived.

In most cases, the connectionEnd that is retrieved using GetConnection is required at some later
step in the function, as such, if a new function is to be added, it should return the open connection end if it exists.

Proposal

Add a new function that encapsulates both these steps; checks for existence and if the connection's state is OPEN, Bikeshed the naming.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@crodriguezvega
Copy link
Contributor

crodriguezvega commented Jul 17, 2023

Let's create an IsOpen function in the Connectionend type that does the open check:

if !connection.IsOpen() {
  return errorsmod.Wrapf(connectiontypes.ErrInvalidConnectionState, "expected %s, got %s", connectiontypes.Open, connectionEnd.State)
}
func (connection ConnectionEnd) IsOpen() bool {
  return connection.State == OPEN
}

@crodriguezvega crodriguezvega added good first issue Good for newcomers and removed needs discussion Issues that need discussion before they can be worked on labels Jul 20, 2023
@DimitrisJim DimitrisJim self-assigned this Jul 24, 2023
@damiannolan
Copy link
Member

Closing based on the comment in #5691 from @crodriguezvega. I think the general consensus has been: explicit is better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done 🥳
Development

No branches or pull requests

3 participants