Skip to content

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

Merged
melekes merged 1 commit intomasterfrom
anton/4837-block_results
Jun 2, 2020
Merged

lite2: correctly return the results of the "latest" block#4931
melekes merged 1 commit intomasterfrom
anton/4837-block_results

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

@melekes
lite2: correctly return the results of the "latest" block
50b9a52 
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 •
edited by melekes
Loading

👋 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-commenter commented Jun 1, 2020

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
cmwaters
cmwaters approved these changes Jun 2, 2020
View reviewed changes
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 •
edited
Loading

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"
  }
}

@egonspace egonspace mentioned this pull request Jul 7, 2020
Merged
5 tasks
@melekes melekes mentioned this pull request Jul 7, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tessr tessr Awaiting requested review from tessr

2 more reviewers

@erikgrinaker erikgrinaker erikgrinaker approved these changes

@cmwaters cmwaters cmwaters approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@melekes melekes

Labels

R:minor Release: Minor

Projects

None yet

Milestone

No milestone

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

@melekes @codecov-commenter @egonspace @erikgrinaker @cmwaters