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 SetConnectionState in Endpoint #3955

Closed
3 tasks
DimitrisJim opened this issue Jun 24, 2023 · 6 comments
Closed
3 tasks

Add SetConnectionState in Endpoint #3955

DimitrisJim opened this issue Jun 24, 2023 · 6 comments
Labels
needs discussion Issues that need discussion before they can be worked on nice-to-have type: code hygiene Clean up code but without changing functionality or interfaces

Comments

@DimitrisJim
Copy link
Contributor

Summary

Similar to #3947, this is a common operation in many tests, can be:

// SetConnectionState sets a connection state
func (endpoint *Endpoint) SetConnectionState(state connectiontypes.State) error {
	connection := endpoint.GetConnection()

	connection.State = state
	endpoint.SetConnection(connection)

	endpoint.Chain.Coordinator.CommitBlock(endpoint.Chain)

	return endpoint.Counterparty.UpdateClient()
}

there's quite a few instances of this in both main and in 04-channel-upgrade.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@DimitrisJim DimitrisJim added needs discussion Issues that need discussion before they can be worked on nice-to-have type: code hygiene Clean up code but without changing functionality or interfaces labels Jun 24, 2023
@chatton
Copy link
Contributor

chatton commented Jun 26, 2023

See this suggestion a similar concept could be applied to the connection.

@colin-axner
Copy link
Contributor

See this suggestion a similar concept could be applied to the connection.

Yay! I like this suggestion, so SetChannel becomes internal and we replace:

chan := endpointA.GetChannel()
chan.State = types.OPEN
endpointA.SetChannel(chan)

with

endpointA.UpdateChannel(func(channel *channeltypes.Channel) {
    channel.State = types.OPEN
})

and

func (endpoint Endpoint) UpdateChannel(mutate func(channel *channeltypes.Channel)) {
    channel := endpoint.GetChannel()
    
    mutate(channel)
    
    endpoint.setChannel(channel)
}

@colin-axner
Copy link
Contributor

Love to see the testing pkg get some design love :)

@DimitrisJim
Copy link
Contributor Author

DimitrisJim commented Jun 28, 2023

ohhh that's a nice suggestion. Though I doubt there's gonna be many additional fields on the Channel (right?) I do think that's a much cleaner approach.

@DimitrisJim
Copy link
Contributor Author

note: going to close this issue at some point and open another with Cians suggestion for both Channel/Connection/anything else.

Would be best to implement and refactor relevant usages after 04-channel-ugprades has been merged.

@DimitrisJim
Copy link
Contributor Author

Closing in preference to #3985

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Issues that need discussion before they can be worked on nice-to-have type: code hygiene Clean up code but without changing functionality or interfaces
Projects
None yet
Development

No branches or pull requests

3 participants