Remove crossings hello

[vuong177]

Description

• Remove param previousChannelID and crossing hellos defense checks in ChanOpenTry
• Remove crossing hello test in testcase

closes: #1311

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Targeted PR against correct branch (see CONTRIBUTING.md)
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the module structure standards.
• Wrote unit and integration tests
• Updated relevant documentation (docs/) or specification (x/<module>/spec/)
• Added relevant godoc comments.
• Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer
• Review Codecov Report in the comment section below once CI passes

[codecov-commenter]

Codecov Report

Merging #1317 (17198d5) into main (6d1fa91) will increase coverage by 0.01%.
The diff coverage is 84.61%.

@@            Coverage Diff             @@
##             main    #1317      +/-   ##
==========================================
+ Coverage   80.04%   80.06%   +0.01%     
==========================================
  Files         164      164              
  Lines       12370    12328      -42     
==========================================
- Hits         9902     9870      -32     
+ Misses       2000     1994       -6     
+ Partials      468      464       -4     

Impacted Files	Coverage Δ
modules/core/04-channel/keeper/handshake.go	91.47% <80.00%> (+2.20%)	⬆️
modules/apps/transfer/ibc_module.go	64.70% <100.00%> (+1.48%)	⬆️
modules/core/keeper/msg_server.go	58.29% <100.00%> (ø)
...les/apps/27-interchain-accounts/host/ibc_module.go	95.00% <0.00%> (-5.00%)	⬇️
modules/apps/transfer/keeper/keeper.go	91.17% <0.00%> (-2.95%)	⬇️

[crodriguezvega]

Nice work! Thanks for opening this PR, @vuong177!

I think we need a changelog entry, I reckon the Improvements section. Although... is this change also state machine breaking, @colin-axner?

[AdityaSripal]

Thanks for the contributions @vuong177 !

[seantking]

Cool! Nice work

[colin-axner]

The application modules need to be adjusted with this change as well

[colin-axner]

I think we need a changelog entry, I reckon the Improvements section. Although... is this change also state machine breaking,

Improvements sounds good. I feel like we should rework out CHANGELOG sections. State machine breaking entries are only useful between minor releases, but I'd prefer this not to get backported

[colin-axner]

Thanks @vuong177 for starting this! Really appreciate the initiative.

Because these changes are critical to core IBC, I'd prefer if an internal developer took over the pr. It can be hard to ensure correctness via reviews and this can be a very tricky pr to ensure all changes are made. It will be easiest/safest done by someone who has a extensive knowledge of the code base

Side note: we also need to update WriteOpenTryChannel

[jackzampolin]

@colin-axner your comment seems to imply that the ibc-go repo isn't open to outside contributions. Is that accurate?

[crodriguezvega]

@colin-axner your comment seems to imply that the ibc-go repo isn't open to outside contributions. Is that accurate?

Colin is on holidays and he will not be able to answer for several weeks, but I will... No, Colin is not implying anything like that at all. We appreciate very much the contributions from external contributors, as this, this, this, this and this show, all PRs from external contributors.

This is a sensitive part of the code and Colin is just worried that something might be overlooked in the review. We welcome the cooperation from external teams, but we also want that to happen in a manner that fits our way of working. By the end of this quarter we will have processes documented to explain how we will do this.

[tac0turtle]

Reading through the comments of this PR, I believe there may be an issue in the process of creating issues. If the team believes a core member should do the work on an issue it should be noted in the issue. According to the issue there was no owner, assignee or mention of who should do the work or not do the work. If the ibc team feels like some issues should not be handled by external contributors then this should be noted in the issue.

[colin-axner]

Hey everyone. Thanks for your patience and I'm sorry for any difficulties which arose from my comments.

We love external contributions. The help from the notional folks and others have been tremendous. It's been great seeing the list of contributors constantly growing ❤️

Thanks @marbar3778 for pointing out the lack of guidance on the issue I created. We should certainly indicate for whom the issue is intended going forward. This repo is about a year old and we are still learning to how to best collaborate with external contributors without causing disruption to the folks who work daily on the repository. Thanks for y'alls patience and feedback, it is heard and appreciated!

As @crodriguezvega mentions, we will work to improve our processes and ensure there is clear documentation on effective ways to work with us

[colin-axner]

Thanks for the contribution @vuong177!