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

Initial BeginBlock contains invalid votes and differs on replay #2810

Closed
KrzysiekJ opened this issue Nov 11, 2018 · 4 comments
Closed

Initial BeginBlock contains invalid votes and differs on replay #2810

KrzysiekJ opened this issue Nov 11, 2018 · 4 comments
Labels
S:waiting Status: Waiting for response

Comments

@KrzysiekJ
Copy link
Contributor

Tendermint version: 0.26.0-c086d0a3

ABCI app: https://ercoin.tech

Steps to reproduce:

  1. Clone the Ercoin repository (current master: 0c6cedda6bdae870a70655).
  2. Install prerequesites as described in README.
  3. Add the following line after line 109 in src/ercoin_abci.erl:
    io:format("Votes at height ~w: ~w~n", [Data#data.height + 1, Votes]),
  1. make app
  2. Generate a testnet by running make testnet and entering the following data options:
#{accounts_opts =>
      [#{address => "OeFZLmb5xrqc60U+/xztZxnVj3SSpKmIWzlz5U83oJU=",
         valid_for => 3600 * 24,
         balance => 100000000000000},
       #{address => "Hdu8UaISmnbiFzTtjagRNJS+FxKi72aUCW0RRsaDIiU=",
         valid_for => 3600 * 24 * 365,
         locked_for => 3600 * 24 * 365,
         balance => 1}],
 epoch_length => 3600}.
  1. Run the ABCI server: make run.
  2. Run Tendermint: tendermint node --home ~/.ercoin/.
  3. Wait a few blocks and stop Tendermint.
  4. Stop ABCI server (Ctrl-C two times).
  5. Run ABCI server again.
  6. Run Tendermint again.

What should happen:

The screen with ABCI server should contain the following info which looks like the following in both invocations (the address bytes will differ):

Votes at height 1: [{'abci.VoteInfo',{'abci.Validator',<<112,152,188,230,92,74,152,172,211,213,119,166,243,244,83,77,51,212,201,252>>,20},true}]

What actually happens:

The screen with ABCI server contains the following info at first invocation:

Votes at height 1: []

It contains the following info at second invocation (note that true is replaced with false):

Votes at height 1: [{'abci.VoteInfo',{'abci.Validator',<<112,152,188,230,92,74,152,172,211,213,119,166,243,244,83,77,51,212,201,252>>,20},false}]

(The above difference doesn’t cause a difference in app hash on replay — and panic as a result — only because currently the pseudotimestamp of last block in initial data is the same as genesis_time — see #2587 and commit 916246e1 in Ercoin).

Have you tried the latest version: yes

@ebuchman
Copy link
Contributor

Note that BeginBlock contains VoteInfo from the LastCommit - ie. the commit for the previous block. For block 1, there is no LastCommit, because we're dealing with the first block. So the list of votes is expected to be empty for the first block.

Does that explain this? Or is something else going on here?

@KrzysiekJ
Copy link
Contributor Author

Thank you, this explains this — previously LastCommitInfo has been treated like current commit info in Ercoin. However, despite this field is meaningless in first block, there is still a discrepancy between what it contains originally and what is sent on replay — which may potentially indicate some more serious problem.

Also sending empty votes in initial LastCommitInfo may aid server implementations. For example, people may want to punish validators reported as absent without the need to check block height.

@ebuchman
Copy link
Contributor

there is still a discrepancy between what it contains originally and what is sent on replay — which may potentially indicate some more serious problem.

Can you clarify the issue here? Could it be related to #2893?

Also sending empty votes in initial LastCommitInfo may aid server implementations. For example, people may want to punish validators reported as absent without the need to check block height.

Not sure what you're asking for here - we should just send the info corresponding to the validator set for this first block, even though all signed_last_block = false ?

@ebuchman ebuchman added the S:waiting Status: Waiting for response label Jan 13, 2019
@ebuchman
Copy link
Contributor

ebuchman commented Feb 8, 2019

Closing this - let us know if there's still something to address.

@ebuchman ebuchman closed this as completed Feb 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S:waiting Status: Waiting for response
Projects
None yet
Development

No branches or pull requests

2 participants