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

lite2: correctly return the results of the "latest" block #4931

Merged
merged 1 commit into from
Jun 2, 2020

Conversation

melekes
Copy link
Contributor

@melekes melekes commented Jun 1, 2020

Closes #4837

  • /block_results

    before:

    failed to update light client to 7: failed to obtain the header Fix p2p/switch.Broadcast impl #7: signed header not found

    after:

    We can't return the latest block results because we won't be able to
    prove them. Return the results for the previous block instead.

  • /block_results?height=X`

    no changes

Closes #4837

- `/block_results`

  before:

  failed to update light client to 7: failed to obtain the header #7: signed header not found

  after:

  We can't return the latest block results because we won't be able to
  prove them. Return the results for the previous block instead.

- /block_results?height=X`

  no changes
@melekes melekes requested a review from tessr as a code owner June 1, 2020 08:22
@auto-comment
Copy link

auto-comment bot commented Jun 1, 2020

👋 Thanks for creating a PR!

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.

  • Wrote tests
  • Updated CHANGELOG_PENDING.md
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments
  • Re-reviewed Files changed in the Github PR explorer
  • Applied Appropriate Labels

Thank you for your contribution to Tendermint! 🚀

@melekes melekes self-assigned this Jun 1, 2020
@melekes melekes added the R:minor Release: Minor label Jun 1, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #4931 into master will decrease coverage by 0.08%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #4931      +/-   ##
==========================================
- Coverage   62.84%   62.75%   -0.09%     
==========================================
  Files         198      198              
  Lines       20047    20055       +8     
==========================================
- Hits        12598    12585      -13     
- Misses       6427     6443      +16     
- Partials     1022     1027       +5     
Impacted Files Coverage Δ
cmd/tendermint/commands/lite.go 24.65% <0.00%> (ø)
lite2/rpc/client.go 0.00% <0.00%> (ø)
consensus/reactor.go 72.64% <0.00%> (-2.33%) ⬇️
blockchain/v0/pool.go 78.02% <0.00%> (-0.64%) ⬇️
p2p/pex/pex_reactor.go 82.12% <0.00%> (ø)
consensus/state.go 74.23% <0.00%> (+0.67%) ⬆️
statesync/snapshots.go 94.07% <0.00%> (+1.48%) ⬆️

@melekes melekes requested a review from cmwaters June 2, 2020 09:54
Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

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

I spun it up and it seemed to work as expected 👍

@melekes melekes merged commit 8ab0a4c into master Jun 2, 2020
@melekes melekes deleted the anton/4837-block_results branch June 2, 2020 14:51
@egonspace
Copy link

egonspace commented Jul 7, 2020

I met following error when I tested it.

curl -s "localhost:8888/block_results"
{
  "jsonrpc": "2.0",
  "id": -1,
  "error": {
    "code": -32603,
    "message": "Internal error",
    "data": "last results  does not match with trusted last results 4837665DFE640A370E7496C691987562D02462142C5F34F59E185911A12370EA"
  }
}

I wonder if this code is right.

// in /light/rpc/client.go: BlockResults()
results := types.NewResults(res.TxsResults)
if rH, tH := results.Hash(), trustedHeader.LastResultsHash; !bytes.Equal(rH, tH)

// LastResultsHash is defined as following
LastResultsHash = ABCIResponsesResultsHash(abciResponses) // merkle.HashFromByteSlices([][]byte{bbeBytes, results.Hash(), ebeBytes})

If TxsResults is nil, then rH is byte[0] but tH is an array having 32 bytes length.

And there's another problem, Validators rpc seems to have not fixed it.

curl -s "localhost:8888/validators"
{
  "jsonrpc": "2.0",
  "id": -1,
  "error": {
    "code": -32603,
    "message": "Internal error",
    "data": "failed to update light client to 1756: failed to obtain the header #1756: signed header not found"
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R:minor Release: Minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lite2: block_results RPC call doesn't correctly return the result of the latest block
5 participants