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

Api doesn't return all blocks from subscribeNewHeads when equivocation happens #1744

Closed
jak-pan opened this issue Jan 24, 2020 · 6 comments
Closed
Labels

Comments

@jak-pan
Copy link
Contributor

jak-pan commented Jan 24, 2020

Not sure if this is polkadot-js/api, substrate related or both. I've got three development OSX machines on local network runing polkadot-kusama nodes trying to test and catch equivocations. My node version is 0.7.19-7c34a67 on all machines polkadot-js api version is 1.0.0-beta.17

My bootstrap validator node (Alice) is running with command

 ./target/release/polkadot   --base-path /tmp/alice   --chain local   --alice   --port 30333   --ws-port 9944   --rpc-port 9933 --validator

command to run equivocating validator nodes (Bob) is

./target/release/polkadot   --base-path /tmp/bob   --chain local   --bob   --port 30333   --ws-port 9944   --rpc-port 9933   --validator --bootnodes /ip4/192.168.0.94/tcp/30333/p2p/QmPUTDXgUonhf46aAXKufWJjnJXzxdGKVaAWEzFeao6wnv

I'm listening to new blocks via

api.rpc.chain.subscribeNewHeads()

When validator nodes Equivocate, Bob1 returns block 0xBob1, Bob2 returns block 0xBob2 Alice receives 0xBob1 or 0xBob2 not both. It also logs this into terminal, while Bob1 and Bob2 act normally.

Slot author Public(8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48 (5FHneW46...)) is equivocating at slot 263316560 with headers 0xBob1 and 0xBob2

Now, if I want to detect equivocation via polkadot-js/api I need to also

api.rpc.chain.subscribeFinalizedHeads()

I need to compare heads on the same height, if I detect different blocks with the same author, since it will also trigger if there was a fork, I can detect equivocation.

The problem with this is it needs to run on all validator instances all the time to be detectable, because it will not trigger for 0xBob2 if it's the finalized block. It would be more predictable if every node received all the blocks just like when fork happens.

Is there any better way to do this right now?

@jak-pan jak-pan changed the title Api returns wrong block from subscribeNewHeads when equivocation happens Api doesn't return all blocks from subscribeNewHeads when equivocation happens Jan 24, 2020
@jacogr
Copy link
Member

jacogr commented Jan 25, 2020

This s not on the API layer itself, the subscription is just a thin wrapper around the RPC - all it does extra is to take the raw data and creates an API type from it.

Even in the cases on forks, you will notice that the node will generally report more forks in the logs than what is received via RPC.

I have had a discussion with @tomusdrw in the past for an additional RPC on Substrate (like subscribeNewHeads), that actually returns everything. The existing RPC is not really the correct avenue to receive everything (which could also include blocks in the past from forks, the current follows the best tip).

@jak-pan
Copy link
Contributor Author

jak-pan commented Jan 25, 2020

Makes sense, had a long night yesterday. Should I close this and log it into substrate?

@jacogr
Copy link
Member

jacogr commented Jan 25, 2020

Let's hear from @tomusdrw to see if there is a need, either already logged or possible to adjust into a new direction first.

@jacogr jacogr added the support label Jan 27, 2020
@tomusdrw
Copy link

The RPC currently only reports blocks that are considered new_best:
https://github.com/paritytech/substrate/blob/2bbdec44cfc2654e59f93a32f91b242bbd538e05/client/rpc/src/chain/mod.rs#L109

I don't think there is an issue logged in the substrate repo to expose "all heads" subscription, but it should be fairly easy to implement (just removing the filter line above and copy & pasting the rest).

We could possibly just parametrize current method (but make it optional to maintain backward compatibility) instead of introducing a new one too.

@jak-pan
Copy link
Contributor Author

jak-pan commented Jan 27, 2020

That would be perfect!

@polkadot-js-bot
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Jun 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants