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

Empty connection id may result in correct result - ConnOpenAck #332

Closed
4 tasks
livelybug opened this issue Oct 20, 2020 · 1 comment · Fixed by #334
Closed
4 tasks

Empty connection id may result in correct result - ConnOpenAck #332

livelybug opened this issue Oct 20, 2020 · 1 comment · Fixed by #334
Assignees
Labels
E: substrate External: substrate integration, requirements & problems

Comments

@livelybug
Copy link

Summary of Bug

  • This is the 3rd step in the connection opening handshake protocol.

  • This is the 3rd step in connection opening handshake, the previous step should have already create the counter-party's connection, and set its state to TRYOPEN. As such, if the counter-party's connection id is empty, it would be an error. However, in ibc-rs, if the counter-party's connection id is empty, the response would be Ok.

// Check that if the msg's counterparty connection id is not empty then it matches
// the old connection's counterparty.
let counterparty_matches = msg.counterparty_connection_id().as_str().is_empty()
|| old_conn_end.counterparty().connection_id() == msg.counterparty_connection_id();

if state_is_consistent && counterparty_matches {
    Ok(old_conn_end.clone())
}

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ancazamfir
Copy link
Collaborator

The counterparty_connection_id has been added to the ConnOpenAck recently as part of the flexible identifier selection. If it is empty then the existing counterparty connection id is used and if all else is good the response will indeed be Ok.
This being said, the proper way to code this is to have msg.counterparty_connection_id as Option<ConnectionId>.

@adizere adizere added the E: substrate External: substrate integration, requirements & problems label Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E: substrate External: substrate integration, requirements & problems
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants