Revert "Include signed ip address in TIER2 handshake. #8902 (#9100)"

[ghost]

This reverts commit bc9ba97 due to https://near.zulipchat.com/#narrow/stream/297663-pagoda.2Fnetwork/topic/network.20stall.20during.20mocknet.20test/near/365948241

[ghost]

Protobuf failures as protobuf backward compatibility doesnt account for git revert ==" but it shouldn't be an issue
https://buildkite.com/nearprotocol/nearcore/builds/28568#0188ba73-7fc8-4ea9-9323-470a35c8eae8

[marcelo-gonzalez]

@saketh-are do you have opinions on the protobuf backward compatibility failure? If I understand correctly, it shouldn't be a big problem right now. Say we have 2 nodes connecting to each other, one before this reversion and one after. Then the one with this reversion might see a proto with a field it doesn't recognize, and will just ignore it silently. The one without the reversion will se a handshake proto without this field and will just populate it w the default, and it should be fine (I tested that it's fine on a localnet setup). But there could be problems down the line if we add another field with the same tag and someone happens to be running a binary that was built from master between when this feature was added and when it was reverted. That's maybe a bit icky but probably not super likely to affect too many people

[saketh-are]

@marcelo-gonzalez That all makes sense, I agree with your analysis. Thanks for thinking through it and testing it.

[ghost]

hey, i'm unable to merge due to buildkite failing because of the protocol buffer incompatibility issue.
I think the only way forward is to git push this commit on the commandline instead of github?
However, I dont think I have git push permissions for the master branch.

Few approaches:
Approach 1: Someone with access git revert this using git commandline directly instead of github UI
Approach 2: We have to temporarily disable protocol buffer compatibility test from running in our automated build kite test, merge this, then reenable the protocol buffer compatibility test
Approach 3: deployment engineers has to simply deploy from this git commit instead of the current master branch and hopefully we get this fixed before the following deployment release ^^ However, the fix itself might have compatibility error to fix as well if this doesn't get merged in our master branch, which may require resorting to approach 1 or 2 if that's the case.

[marcelo-gonzalez]

hey, i'm unable to merge due to buildkite failing because of the protocol buffer incompatibility issue. I think the only way forward is to git push this commit on the commandline instead of github? However, I dont think I have git push permissions for the master branch.

Few approaches: Approach 1: Someone with access git revert this using git commandline directly instead of github UI Approach 2: We have to temporarily disable protocol buffer compatibility test from running in our automated build kite test, merge this, then reenable the protocol buffer compatibility test Approach 3: deployment engineers has to simply deploy from this git commit instead of the current master branch and hopefully we get this fixed before the following deployment release ^^ However, the fix itself might have compatibility error to fix as well if this doesn't get merged in our master branch, which may require resorting to approach 1 or 2 if that's the case.

@soonnear for now I've just cherry picked this into my 1.35.0 branch: https://github.com/marcelo-gonzalez/nearcore/tree/1.35.0. And maybe for committing this to master, we should update the buildkite backwards compatibility test to be more flexible? As in to allow an explicit ACK from the author/reviewer that we're okay with the breaking change. And another option is to just reserve the tag, and use the tag 10 for any new field there.

[ghost]

@marcelo-gonzalez Thanks, seems like reserving that 1 number is the quickest way to get this merged.

Numbers can be pretty high.

• https://protobuf.dev/programming-guides/proto3/

Although extremely unlikely, if protobuf performance ever becomes a concern in future that requires all field numbers to be <= 15, a migration to a new protobuf message can be an option.