search

hashgraph / hedera-sdk-java

Hedera™ Hashgraph SDK for Java
https://docs.hedera.com/guides/docs/sdks
Apache License 2.0
227 stars 117 forks source link

`Client.setNetworkFromAddressBook(NodeAddressBook)` does not set certificate information #1995

Closed steven-sheehy closed 1 month ago

steven-sheehy commented 1 month ago

Description

Client.setNetworkFromAddressBook(NodeAddressBook) is both a public method that can be called directly by the user and also used internally when the address book is refreshed automatically from the mirror node. The certificate info inside the NodeAddressBook parameter is not passed through to Node.setAddressBookEntry() so TLS does not work unless we also specify Client.setVerifyCertificates(false), which is less secure.

Not tested, but believe it also results in the refreshed address book clearing the cert hash info from the built-in address book.

Steps to reproduce

Invoke Client.setNetworkFromAddressBook(NodeAddressBook) with a NodeAddressBook that has a list of consensus nodes each with 50212 TLS port and cert hash info.

Additional context

No response

Hedera network

testnet

Version

2.38.0

Operating system

None

0xivanov commented 1 month ago

Hey @steven-sheehy . Can you check out https://github.com/hashgraph/hedera-sdk-java/pull/1901 ? The newly added overload of setNetworkFromAddressBook() might be fixing the issue.

steven-sheehy commented 1 month ago

@0xivanov Yes, that might meet our needs. But I'm not sure why it was made optional and off by default. In what scenarios would you update the address book but want to omit updating the nodes' certificate hash? They should always stay in sync. Especially for the automated refresh since users don't even have a way to opt into updating the cert hash that way. IMO, we should remove the updateAddressBook overloaded method and always update.

0xivanov commented 1 month ago

We decided to make it optional just to make sure we are not breaking something. Is it possible for the mirror node team to test using the new function just to be sure everything works? We could remove the overloaded method in one of the next releases if it's Ok.

0xivanov commented 1 month ago

Closing and adding a follow-up issue to enable this by default. https://github.com/hashgraph/hedera-sdk-java/issues/2015