[Shipped] A Multi-View Solution for PBFT Protocol

Hi @team

I’m pre-reviewing base on this pull request. With 158 file and 188 commits, I think that I need to spend a lot of days to do it :smile:. We know that this is a pre-review for multi-view solution coding, not final coding because you guys need to merge portal v2 for a finalized pull request. I will send some reviewing code for 1st fixing:

1/ We can remove case data := <-cm.data here, right?
image
because this function is no longer used
image

2/ shardID come from RPC in some case. So I think we need to check index of slice and nil pointer exception
image
because if RPC is invalid, the node will be crashed

3/ This function always return 0 -> remove it?
image
and review where we use it
image

4/ Singleton in beacon pool
image
We can use blockchainObj as a field in BeaconPool singleton, right? Because any node also needs to get beacon, singleton beacon pool will handle this blockchainObj and shardpool can reuse it

5/ Many interface of server object is no longer used
image
image
image
can we remove all of them?

6/ I think 3 functions are not used, right?
image

7/ Remove this condition
image
image

9 Likes
  • RPC must check this is parameter, if not it’s very carelessness
  • Because in paramter is byte. It’s a value type range from 0->255 so it can’t be nil in this case. And also there’s a condition checking BestView is exist or not before getting it. So i think this function is safe
3 Likes

As i recalled beaconpool/shardpool/crossshardpool is outdated, no longer in use. @0xkumi plz very my answer

3 Likes

We will not use singleton pool in this version.

2 Likes

Weekly update 18 - 24 April (in milestone “Run multiview for consensus v1 on testnet”)

  • After discussing with the pdex bridge team, we decided that the current version is safe to deploy. As the fixed proposer mechanism will prevent forks, at the current stage, we will not care much in revert cases.
  • @hungngo worked on partition database for each chain (shard, beacon), so that we could backup easily a certain chain.
  • We continue to implement a feature on Highway that deliberately makes fork cases based on the predefined scenario.
  • This week, we reviewed and merged new portal features to our code. We also fixed some minor bugs about concurrency and sinker.

Next week, we want to test again the code with other features before deploying on testnet.

5 Likes

Hi @team

I’m working to review 2nd for multiview on pull request 880(after your team merged portal v2 for a final pull request). And I have some trouble with ChainInterface. I know that we have blsbft for fixed node version and blsbftv2 for non-fixed node version. So we have 2 interface here
blsbft
image
blsbftv2
image

But, the fact that I also have a ChainInterface in consensus folder, for What?
image
This is no need for our design, right?

3 Likes

And by the way, in case we only use ChainInterface of blsbft folder. So, where is function GetPubkeyRole
image
In my memory, I remember this function is used for some function of pNode of app team for checking staking node. If this function is removed, you guys checked to related feature on RPC, right?

2 Likes

ChainInterface in consensus/interface.go is no longer is used, @0xkumi plz confirm this

1 Like

Hi @team,

We complete reivew PR 880. Tomorrow, we will deploy this on testnet

Congratulation!

2 Likes

Weekly update 25 April - 1 May (in milestone “Run multiview for consensus v1 on testnet”)

  • We are ready to launch multiview on testnet this week, however, there is a delay due to holiday time.
  • In May, the team will work on the next milestone which we allow to round-robin the proposer (consensus v2). One step closer to decentralize our network.
  • Regard to team resource, @dungtran and @lam will not join in next phase as they will research on another topic.
4 Likes

I’m running some validator node with this multiview solution by docker tag consensus_multiview_20200504_1 on testnet. Because we have some change in state of beacon and shard, this version should sync testnet data again. When these node is ready for consensus, I will deploy for all testnet node. Hope these will be synced full data of testnet on tomorrow and we can do that on tomorrow afternoon

Thanks

1 Like

Hi @0xkumi @hungngo and team,
I tried to run a node with multiview and realized that it did not sync the data of shard 2, although this node is a validator of shard 2.

Please look into this issue.

Thanks

1 Like

Hi @0xkumi and team

We have an issue with portal V2, can not sync data, so we will delay deploy consensus-v2 -multiview on testnet and run an old version with portal v2 and sync from scratch again with code of portal v2 to make sure this is not an issue of your team.

I will notice later.

Thanks

1 Like

Hi @team, with your consensus-v2 multiview, I can sync data of testnet. But we have a new issue that validator can not join consensus and send vote, it only receive other vote
image

Please look over this issue and fix it

Thanks

1 Like

Thank you @thaibao we have issue with new database layout. And already fixed it.

Weekly update 2 May - 8 May (in milestone “Run multiview for consensus v2 on testnet”)

  • This week, we have some issues of multiview protocol related to database and fork logic. They all fixed and we are re-run on testnet.
  • QC team also start to test consensus v2 which allows round-robin proposer.
  • The fork case simulation tool also finish this week.

Next week, we will focus on testing consensus v2 code and run simulation cases.

2 Likes

The fact is, I could run your latest code on 1 node of each shard in the testnet. It synced perfectly and joined consensus. But this is only one node in a shard. We need to replace all nodes in testnet with this code and unfortunately, we have some issues around portal v2 which I noticed. We need to:

  • Try syncing a fullnode in testnet with fully verified block
  • Wait for portal v2 bugs to be fix and replace all nodes in testnet

Thanks.

1 Like

We have an issue with deposit token on testnet when I test by changing beacons with 1/3 node is multiview. Beacon can not create block with deposit token bridge

image

That means
1/ beacon 0(old code) create block with deposit tx
2/ beacon 1(old code) verify that ok and send vote
3/ beacon 3(multiview code) verify fail

-> Block can not be created.

Please look into this issue.

1 Like

Hey @0xkumi, for the April work, your project got 3/5 votes, resulting in a 50% funding released. PRV should have arrived at your wallet by now. Please kindly check!

Great month ahead!!

Update (in milestone “Run multiview for consensus v2 on testnet”)

  • This week, we didnot develop consensus v2 anymore, but focus on fix bug reported from QC team.
  • We also started to simulate some fork scenarios in local machine.
  • From this week, team will focus on making the network more robust and reliable, which will be shown in another thread.
3 Likes

For a good early publishment this feature on mainnet and prepare better for multiview bft-2 solution (with round robind block proposor). We have plan publish multiview with engine bft-1(still fix block proposor) in this July on mainnet.

About multiview bft-2, we’re working in new proposal Improve Node Performance

5 Likes