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

fix(tendermint-rpc): add earliest block to sync info #1449

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

livthomas
Copy link
Contributor

This should fix issue #1448. Those fields are returned from /status endpoint but I haven't tested this code. I'm open to any suggestions how I can test it easily.

@webmaster128
Copy link
Member

See

for the place to put the tests. This is tested against a real Tendermint instance in CI

@livthomas
Copy link
Contributor Author

@webmaster128 I've added a few unit tests there. But it looks like the tests are failing because those new fields are missing. What nodes does the client connect to when it is being tested in CI?

These fields have been added to Tendermint more than three years ago. So either there is some really old node running in CI or mock data without these fields is used there. But I don't see it defined anywhere in the codebase.

@webmaster128
Copy link
Member

Thew CI runs Tendermint version 0.34 and 0.37.

Looking at the code you linked there it seems like the fields may not be set if the metadata about the earliest block is missing
Bildschirmfoto 2023-06-29 um 23 30 45

So all 4 fields are probably optional and some sort of unset/default/zero value. Could you just dump status.syncInfo and check what you actually get from the node?

@livthomas
Copy link
Contributor Author

I've printed it to the console and it looks like the problem is only with earliest_app_hash sometimes being an empty string in the tests.

Original data:

➤ YN0000: [@cosmjs/tendermint-rpc]: {
➤ YN0000: [@cosmjs/tendermint-rpc]:   latest_block_hash: '48EEF4251889F64378A6AA42150D99BA405D6068273A488002522622BF373A8A',
➤ YN0000: [@cosmjs/tendermint-rpc]:   latest_app_hash: '2800000000000000',
➤ YN0000: [@cosmjs/tendermint-rpc]:   latest_block_height: '483',
➤ YN0000: [@cosmjs/tendermint-rpc]:   latest_block_time: '2023-06-30T07:43:44.73368918Z',
➤ YN0000: [@cosmjs/tendermint-rpc]:   earliest_block_hash: 'AA6E23C961D3B22A54227E84387D0830D6E28F838E4367F0529E4BFE891AEDEE',
➤ YN0000: [@cosmjs/tendermint-rpc]:   earliest_app_hash: '',
➤ YN0000: [@cosmjs/tendermint-rpc]:   earliest_block_height: '1',
➤ YN0000: [@cosmjs/tendermint-rpc]:   earliest_block_time: '2023-06-30T07:39:32.998341288Z',
➤ YN0000: [@cosmjs/tendermint-rpc]:   catching_up: false
➤ YN0000: [@cosmjs/tendermint-rpc]: }

Processed object:

➤ YN0000: [@cosmjs/tendermint-rpc]: {
➤ YN0000: [@cosmjs/tendermint-rpc]:   earliestAppHash: undefined,
➤ YN0000: [@cosmjs/tendermint-rpc]:   earliestBlockHash: Uint8Array(32) [
➤ YN0000: [@cosmjs/tendermint-rpc]:     170, 110,  35, 201,  97, 211, 178,  42,
➤ YN0000: [@cosmjs/tendermint-rpc]:      84,  34, 126, 132,  56, 125,   8,  48,
➤ YN0000: [@cosmjs/tendermint-rpc]:     214, 226, 143, 131, 142,  67, 103, 240,
➤ YN0000: [@cosmjs/tendermint-rpc]:      82, 158,  75, 254, 137,  26, 237, 238
➤ YN0000: [@cosmjs/tendermint-rpc]:   ],
➤ YN0000: [@cosmjs/tendermint-rpc]:   earliestBlockHeight: 1,
➤ YN0000: [@cosmjs/tendermint-rpc]:   earliestBlockTime: 2023-06-30T07:39:32.998Z { nanoseconds: 341288 },
➤ YN0000: [@cosmjs/tendermint-rpc]:   latestBlockHash: Uint8Array(32) [
➤ YN0000: [@cosmjs/tendermint-rpc]:      80, 116, 225, 190, 231, 174, 91, 203,
➤ YN0000: [@cosmjs/tendermint-rpc]:      45, 135,  87, 222,  62, 177, 92, 157,
➤ YN0000: [@cosmjs/tendermint-rpc]:      37,  62,  63, 187,  30,  38, 48, 161,
➤ YN0000: [@cosmjs/tendermint-rpc]:     190,  61, 174,  67, 104, 115, 15,  24
➤ YN0000: [@cosmjs/tendermint-rpc]:   ],
➤ YN0000: [@cosmjs/tendermint-rpc]:   latestAppHash: Uint8Array(8) [
➤ YN0000: [@cosmjs/tendermint-rpc]:     34, 0, 0, 0,
➤ YN0000: [@cosmjs/tendermint-rpc]:      0, 0, 0, 0
➤ YN0000: [@cosmjs/tendermint-rpc]:   ],
➤ YN0000: [@cosmjs/tendermint-rpc]:   latestBlockTime: 2023-06-30T07:43:43.174Z { nanoseconds: 329302 },
➤ YN0000: [@cosmjs/tendermint-rpc]:   latestBlockHeight: 480,
➤ YN0000: [@cosmjs/tendermint-rpc]:   catchingUp: false
➤ YN0000: [@cosmjs/tendermint-rpc]: }

But there are other tests where it is defined properly:

➤ YN0000: [@cosmjs/ledger-amino]: {
➤ YN0000: [@cosmjs/ledger-amino]:   latest_block_hash: '21C487313F76F8277E010CC8EB3C4B4DE7460F5836312B97B901EC1A729BB8C7',
➤ YN0000: [@cosmjs/ledger-amino]:   latest_app_hash: '62AA52D1754B7F318CE1D25CF6F1D5826F9CA98A4E14A4B4C54D0BF315394C2E',
➤ YN0000: [@cosmjs/ledger-amino]:   latest_block_height: '424',
➤ YN0000: [@cosmjs/ledger-amino]:   latest_block_time: '2023-06-30T07:46:41.697890244Z',
➤ YN0000: [@cosmjs/ledger-amino]:   earliest_block_hash: '2BE0795DF0BE90E0851D0A978EA34C28CEC5F0218465F230AE16AF460C38E5F8',
➤ YN0000: [@cosmjs/ledger-amino]:   earliest_app_hash: 'E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855',
➤ YN0000: [@cosmjs/ledger-amino]:   earliest_block_height: '1',
➤ YN0000: [@cosmjs/ledger-amino]:   earliest_block_time: '2023-04-06T15:56:56.752766256Z',
➤ YN0000: [@cosmjs/ledger-amino]:   catching_up: false
➤ YN0000: [@cosmjs/ledger-amino]: }

Or:

➤ YN0000: [@cosmjs/faucet]: {
➤ YN0000: [@cosmjs/faucet]:   latest_block_hash: '716D48C3BB099E7EBF8A454AD3C51F545707F9150326AE6D02E21C750D5FC4CF',
➤ YN0000: [@cosmjs/faucet]:   latest_app_hash: 'F7EFE02655C78B68B66D0C7C34DF97D1AEF895ABB4A12F98244C3BF236B28AE4',
➤ YN0000: [@cosmjs/faucet]:   latest_block_height: '414',
➤ YN0000: [@cosmjs/faucet]:   latest_block_time: '2023-06-30T07:46:31.491128123Z',
➤ YN0000: [@cosmjs/faucet]:   earliest_block_hash: '2BE0795DF0BE90E0851D0A978EA34C28CEC5F0218465F230AE16AF460C38E5F8',
➤ YN0000: [@cosmjs/faucet]:   earliest_app_hash: 'E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855',
➤ YN0000: [@cosmjs/faucet]:   earliest_block_height: '1',
➤ YN0000: [@cosmjs/faucet]:   earliest_block_time: '2023-04-06T15:56:56.752766256Z',
➤ YN0000: [@cosmjs/faucet]:   catching_up: false
➤ YN0000: [@cosmjs/faucet]: }

When looking at the latest implementation of status endpoint, I can see there might be Golang zero values returned from it unless earliestBlockMeta is set. I'll change the code to detect these values and return undefined in such cases.

@livthomas livthomas force-pushed the fix/rpc-earliest-block branch 2 times, most recently from 84ab4aa to c813712 Compare June 30, 2023 09:05
@livthomas
Copy link
Contributor Author

@webmaster128 Can we merge this or is there something else I should do?

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Could you add a CHANGLOG.md entry to the Unreleadsed section please?

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@webmaster128 webmaster128 merged commit 358260b into cosmos:main Jul 11, 2023
12 checks passed
@livthomas livthomas deleted the fix/rpc-earliest-block branch July 11, 2023 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants