Repository: pactus-project/pactus
Pull request #806 (2023-11-09 10:31:06)

fix(execution): fixing mistake on calculating unbonded power
## Description

This PR fixes a mistake in calculating unbonded power. 
It is important to note that the power for an unbonded validator is zero. 
Therefore, when calculating the [powerDelta](https://github.com/pactus-project/pactus/blob/02bff51ade9d7c89a72653f59670e6c3b3c50c77/sandbox/sandbox.go#L264), we should consider the stake, not the power.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pactus-project/pactus/pull/806?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pactus-project) Report
> Merging [#806](https://app.codecov.io/gh/pactus-project/pactus/pull/806?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pactus-project) (ab722d2) into [main](https://app.codecov.io/gh/pactus-project/pactus/commit/02bff51ade9d7c89a72653f59670e6c3b3c50c77?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pactus-project) (02bff51) will **not change** coverage.
> The diff coverage is `100.00%`.


```diff
@@           Coverage Diff           @@
##             main     #806   +/-   ##
=======================================
  Coverage   82.98%   82.98%           
=======================================
  Files         173      173           
  Lines        8253     8253           
=======================================
  Hits         6849     6849           
- Misses       1084     1085    +1     
+ Partials      320      319    -1     
```



Patch:
diff --git a/execution/executor/unbond.go b/execution/executor/unbond.go
index 8bc45f02..b1ebd2a4 100644
--- a/execution/executor/unbond.go
+++ b/execution/executor/unbond.go
@@ -48,12 +48,13 @@ func (e *UnbondExecutor) Execute(trx *tx.Tx, sb sandbox.Sandbox) error {
 		}
 	}
 
+	unboundedPower := val.Power()
 	val.UpdateUnbondingHeight(sb.CurrentHeight())
 
 	// At this point, the validator's power is zero.
 	// However, we know the validator's stake.
 	// So, we can update the power delta with the negative of the validator's stake.
-	sb.UpdatePowerDelta(-1 * val.Power())
+	sb.UpdatePowerDelta(-1 * unboundedPower)
 	sb.UpdateValidator(val)
 
 	return nil
diff --git a/sandbox/sandbox.go b/sandbox/sandbox.go
index effec14b..3965f5ac 100644
--- a/sandbox/sandbox.go
+++ b/sandbox/sandbox.go
@@ -281,6 +281,8 @@ func (sb *sandbox) VerifyProof(blockHeight uint32, proof sortition.Proof, val *v
 	if err != nil {
 		return false
 	}
+	// TODO: improvement:
+	// We can get the sortition seed without parsing the block
 	blk, err := committedBlock.ToBlock()
 	if err != nil {
 		return false
diff --git a/sync/sync.go b/sync/sync.go
index 41c8f396..8078141e 100644
--- a/sync/sync.go
+++ b/sync/sync.go
@@ -198,15 +198,15 @@ func (sync *synchronizer) receiveLoop() {
 }
 
 func (sync *synchronizer) processConnectEvent(ce *network.ConnectEvent) {
+	sync.peerSet.UpdateStatus(ce.PeerID, peerset.StatusCodeConnected)
+	sync.peerSet.UpdateAddress(ce.PeerID, ce.RemoteAddress)
+
 	if ce.SupportStream {
 		if err := sync.sayHello(ce.PeerID); err != nil {
 			sync.logger.Warn("sending Hello message failed",
 				"to", ce.PeerID, "error", err)
 		}
 	}
-
-	sync.peerSet.UpdateStatus(ce.PeerID, peerset.StatusCodeConnected)
-	sync.peerSet.UpdateAddress(ce.PeerID, ce.RemoteAddress)
 }
 
 func (sync *synchronizer) processDisconnectEvent(de *network.DisconnectEvent) {


Test patch:
diff --git a/execution/executor/bond_test.go b/execution/executor/bond_test.go
index 1230d5df..88de2bbb 100644
--- a/execution/executor/bond_test.go
+++ b/execution/executor/bond_test.go
@@ -170,3 +170,22 @@ func TestStakeExceeded(t *testing.T) {
 	err := exe.Execute(trx, td.sandbox)
 	assert.Equal(t, errors.Code(err), errors.ErrInvalidAmount)
 }
+
+func TestPwerDeltaBond(t *testing.T) {
+	td := setup(t)
+	exe := NewBondExecutor(true)
+
+	senderAddr, senderAcc := td.sandbox.TestStore.RandomTestAcc()
+	senderBalance := senderAcc.Balance()
+	pub, _ := td.RandBLSKeyPair()
+	receiverAddr := pub.ValidatorAddress()
+	amt, fee := td.randomAmountAndFee(td.sandbox.TestParams.MinimumStake, senderBalance)
+	lockTime := td.sandbox.CurrentHeight()
+	trx := tx.NewBondTx(lockTime, senderAddr,
+		receiverAddr, pub, amt, fee, "ok")
+
+	err := exe.Execute(trx, td.sandbox)
+	assert.NoError(t, err, "Ok")
+
+	assert.Equal(t, amt, td.sandbox.PowerDelta())
+}
diff --git a/execution/executor/unbond_test.go b/execution/executor/unbond_test.go
index 3eafe727..31094f05 100644
--- a/execution/executor/unbond_test.go
+++ b/execution/executor/unbond_test.go
@@ -12,9 +12,16 @@ func TestExecuteUnbondTx(t *testing.T) {
 	td := setup(t)
 	exe := NewUnbondExecutor(true)
 
+	bonderAddr, bonderAcc := td.sandbox.TestStore.RandomTestAcc()
+	bonderBalance := bonderAcc.Balance()
+	stake, _ := td.randomAmountAndFee(td.sandbox.TestParams.MinimumStake, bonderBalance)
+	bonderAcc.SubtractFromBalance(stake)
+	td.sandbox.UpdateAccount(bonderAddr, bonderAcc)
+
 	pub, _ := td.RandBLSKeyPair()
 	valAddr := pub.ValidatorAddress()
 	val := td.sandbox.MakeNewValidator(pub)
+	val.AddToStake(stake)
 	td.sandbox.UpdateValidator(val)
 	lockTime := td.sandbox.CurrentHeight()
 
@@ -53,7 +60,7 @@ func TestExecuteUnbondTx(t *testing.T) {
 		assert.Error(t, err)
 	})
 
-	assert.Zero(t, td.sandbox.Validator(valAddr).Stake())
+	assert.Equal(t, stake, td.sandbox.Validator(valAddr).Stake())
 	assert.Zero(t, td.sandbox.Validator(valAddr).Power())
 	assert.Equal(t, td.sandbox.Validator(valAddr).UnbondingHeight(), td.sandbox.CurrentHeight())
 	assert.Equal(t, td.sandbox.PowerDelta(), -1*val.Stake())
@@ -97,3 +104,22 @@ func TestUnbondJoiningCommittee(t *testing.T) {
 	assert.Error(t, exe1.Execute(trx, td.sandbox))
 	assert.NoError(t, exe2.Execute(trx, td.sandbox))
 }
+
+func TestPwerDeltaUnbond(t *testing.T) {
+	td := setup(t)
+	exe := NewUnbondExecutor(true)
+
+	pub, _ := td.RandBLSKeyPair()
+	valAddr := pub.ValidatorAddress()
+	val := td.sandbox.MakeNewValidator(pub)
+	amt := td.RandInt64(1e9)
+	val.AddToStake(amt)
+	td.sandbox.UpdateValidator(val)
+	lockTime := td.sandbox.CurrentHeight()
+	trx := tx.NewUnbondTx(lockTime, valAddr, "Ok")
+
+	err := exe.Execute(trx, td.sandbox)
+	assert.NoError(t, err)
+
+	assert.Equal(t, -1*amt, td.sandbox.PowerDelta())
+}
diff --git a/sync/sync_test.go b/sync/sync_test.go
index d36972d7..72bd3558 100644
--- a/sync/sync_test.go
+++ b/sync/sync_test.go
@@ -232,7 +232,6 @@ func TestConnectEvents(t *testing.T) {
 		}
 		td.shouldPublishMessageWithThisType(t, td.network, message.TypeHello)
 		p := td.sync.peerSet.GetPeer(pid)
-		require.NotNil(t, p)
 		assert.Equal(t, p.Status, peerset.StatusCodeConnected)
 		assert.Equal(t, p.Address, "address_1")
 	})
@@ -246,7 +245,6 @@ func TestConnectEvents(t *testing.T) {
 		}
 		td.shouldNotPublishMessageWithThisType(t, td.network, message.TypeHello)
 		p := td.sync.peerSet.GetPeer(pid)
-		require.NotNil(t, p)
 		assert.Equal(t, p.Status, peerset.StatusCodeConnected)
 		assert.Equal(t, p.Address, "address_1")
 	})