wemixarchive / go-wemix

Go implementation of the Wemix project.
https://www.wemix.com/
GNU Lesser General Public License v3.0
28 stars 25 forks source link

feat: brioche hard fork; halving block reward #75

Closed egonspace closed 6 months ago

egonspace commented 6 months ago

Brioche hard fork includes the following.

And added some refactoring about validation for Fees and Reward fields of a block.

The following tested manually.

jed-wemade commented 6 months ago

There is no Brioche config validation whether configs are same among the nodes. Without the validation, the below test vector would be passed.

func TestRewardValidation(t *testing.T) {
    ...
    byzantineConfig := &params.ChainConfig{
        ChainID:      gspec.Config.ChainID,
        LondonBlock:  gspec.Config.LondonBlock,
        BriocheBlock: gspec.Config.BriocheBlock,
        Brioche: &params.BriocheConfig{
            BlockReward:       gspec.Config.Brioche.BlockReward,
            FirstHalvingBlock: gspec.Config.Brioche.FirstHalvingBlock,
            HalvingPeriod:     new(big.Int).Div(gspec.Config.Brioche.HalvingPeriod, big.NewInt(2)), // different period!!
            FinishRewardBlock: gspec.Config.Brioche.FinishRewardBlock,
            HalvingTimes:      gspec.Config.Brioche.HalvingTimes,
            HalvingRate:       gspec.Config.Brioche.HalvingRate,
        }
    }

Node cannot catch the difference before applying config from correct nodes. Hence, nodes set by wrong config params would compromise the safety.

egonspace commented 6 months ago

There is no Brioche config validation whether configs are same among the nodes. Without the validation, the below test vector would be passed.

func TestRewardValidation(t *testing.T) {
  ...
  byzantineConfig := &params.ChainConfig{
      ChainID:      gspec.Config.ChainID,
      LondonBlock:  gspec.Config.LondonBlock,
      BriocheBlock: gspec.Config.BriocheBlock,
      Brioche: &params.BriocheConfig{
          BlockReward:       gspec.Config.Brioche.BlockReward,
          FirstHalvingBlock: gspec.Config.Brioche.FirstHalvingBlock,
          HalvingPeriod:     new(big.Int).Div(gspec.Config.Brioche.HalvingPeriod, big.NewInt(2)), // different period!!
          FinishRewardBlock: gspec.Config.Brioche.FinishRewardBlock,
          HalvingTimes:      gspec.Config.Brioche.HalvingTimes,
          HalvingRate:       gspec.Config.Brioche.HalvingRate,
      }
  }

Node cannot catch the difference before applying config from correct nodes. Hence, nodes set by wrong config params would compromise the safety.

Unfortunately, we don't have a mechanism to compare configurations with other nodes. But if you think about it, you can see that synchronizing configurations between nodes is not conceptually correct. This is because, for example, different config settings are indispensable for nodes during the hard fork upgrade.

jed-wemade commented 6 months ago

Unfortunately, we don't have a mechanism to compare configurations with other nodes. But if you think about it, you can see that synchronizing configurations between nodes is not conceptually correct. This is because, for example, different config settings are indispensable for nodes during the hard fork upgrade.

I got it. It is general hard fork situation. I was concern different configs make same state among nodes for a while. The config affects on future halving blocks, and fork will be done by major nodes.

egonspace commented 6 months ago

Unfortunately, we don't have a mechanism to compare configurations with other nodes. But if you think about it, you can see that synchronizing configurations between nodes is not conceptually correct. This is because, for example, different config settings are indispensable for nodes during the hard fork upgrade.

I got it. It is general hard fork situation. I was concern different configs make same state among nodes for a while. The config affects on future halving blocks, and fork will be done by major nodes.

According to your good advice, if the next fork information is different when handshaking with peer, I left a logging so that we could prepare for it in advance.

WARN [05-28|17:28:03.345] Checking ForkID: different next fork     peer=f6215791327f1b5a52a763297a5263407a92ea2fa586a7b887410ef9e5b6800d peerForkID="{Hash:[44 77 205 215] Next:0}" myForkID="{Hash:[44 77 205 215] Next:607147}" error=nil