311 review test case ssendpopresultoutbytes true todo bug (#313)

* added PoP report validation to the ReportPoPResult msg server
* removed two obsolete pop report deliveries
* added challenger is the PoPreport msg creator (new restriction)
* extended msg server test cases

---------

Signed-off-by: Jürgen Eckel <juergen@riddleandcode.com>

tests/e2e/dao/pop_participant_selection_suite.go

@@ -138,8 +138,16 @@ func (s *PopSelectionE2ETestSuite) sendPoPResult(storedChallenge []byte, success
 	challenge.Finished = true
 	challenge.Success = success
 
-	msg := daotypes.NewMsgReportPopResult(val.Address.String(), &challenge)
+	machineName := machines[0].name
-	_, err = e2etestutil.BuildSignBroadcastTx(s.T(), val.Address, msg)
+	if challenge.Challenger != machines[0].address {
+		machineName = machines[1].name
+	}
+	k, err := val.ClientCtx.Keyring.Key(machineName)
+	s.Require().NoError(err)
+	challengerAccAddress, _ := k.GetAddress()
+
+	msg := daotypes.NewMsgReportPopResult(challengerAccAddress.String(), &challenge)
+	_, err = e2etestutil.BuildSignBroadcastTx(s.T(), challengerAccAddress, msg)
 	s.Require().NoError(err)
 }
 
@@ -148,7 +156,6 @@ func (s *PopSelectionE2ETestSuite) TestPopSelectionNoActors() {
 
 	assert.NotContains(s.T(), out.String(), machines[0].address)
 	assert.NotContains(s.T(), out.String(), machines[1].address)
-	s.sendPoPResult(out.Bytes(), true)
 }
 
 func (s *PopSelectionE2ETestSuite) TestPopSelectionOneActors() {
@@ -159,7 +166,6 @@ func (s *PopSelectionE2ETestSuite) TestPopSelectionOneActors() {
 
 	assert.NotContains(s.T(), out.String(), machines[0].address)
 	assert.NotContains(s.T(), out.String(), machines[1].address)
-	s.sendPoPResult(out.Bytes(), true)
 }
 
 func (s *PopSelectionE2ETestSuite) TestPopSelectionTwoActors() {

x/dao/keeper/msg_server_report_pop_result.go

@@ -10,6 +10,26 @@ import (
 	"github.com/planetmint/planetmint-go/x/dao/types"
 )
 
+func (k msgServer) updateChallenge(ctx sdk.Context, msg *types.MsgReportPopResult) (err error) {
+	challenge, found := k.LookupChallenge(ctx, msg.GetChallenge().GetHeight())
+	if !found {
+		err = errorsmod.Wrapf(types.ErrInvalidChallenge, "no challenge found for PoP report")
+		return
+	}
+	if challenge.Challengee != msg.GetChallenge().Challengee ||
+		challenge.Challenger != msg.GetChallenge().Challenger ||
+		challenge.Initiator != msg.GetChallenge().Initiator ||
+		challenge.Height != msg.GetChallenge().Height {
+		err = errorsmod.Wrapf(types.ErrInvalidChallenge, "PoP report data does not match challenge")
+		return
+	}
+	challenge.Success = msg.GetChallenge().GetSuccess()
+	challenge.Finished = true
+
+	k.StoreChallenge(ctx, challenge)
+	return
+}
+
 func (k msgServer) ReportPopResult(goCtx context.Context, msg *types.MsgReportPopResult) (*types.MsgReportPopResultResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
@@ -17,8 +37,18 @@ func (k msgServer) ReportPopResult(goCtx context.Context, msg *types.MsgReportPo
 	if err != nil {
 		return nil, errorsmod.Wrapf(types.ErrInvalidChallenge, err.Error())
 	}
-	// ensure the challenge is stored even without the token reward minting
+
-	k.StoreChallenge(ctx, *msg.Challenge)
+	// verify that the report origin is the challenger
+	if msg.GetCreator() != msg.GetChallenge().GetChallenger() {
+		err = errorsmod.Wrapf(types.ErrInvalidPopReporter, "PoP reporter is not the challenger")
+		return nil, err
+	}
+
+	// update valid PoP Result reports
+	err = k.updateChallenge(ctx, msg)
+	if err != nil {
+		return nil, err
+	}
 
 	if msg.Challenge.GetSuccess() {
 		util.GetAppLogger().Info(ctx, "PoP at height %v was successful", msg.Challenge.GetHeight())

x/dao/keeper/msg_server_test.go

@@ -14,14 +14,14 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func setupMsgServer(t testing.TB) (types.MsgServer, context.Context) {
+func setupMsgServer(t testing.TB) (types.MsgServer, context.Context, *keeper.Keeper) {
 	k, ctx := keepertest.DaoKeeper(t)
-	return keeper.NewMsgServerImpl(*k), sdk.WrapSDKContext(ctx)
+	return keeper.NewMsgServerImpl(*k), sdk.WrapSDKContext(ctx), k
 }
 
 func TestMsgServer(t *testing.T) {
 	t.Parallel()
-	ms, ctx := setupMsgServer(t)
+	ms, ctx, _ := setupMsgServer(t)
 	require.NotNil(t, ms)
 	require.NotNil(t, ctx)
 }
@@ -60,7 +60,7 @@ func TestMsgServerReportPoPResult(t *testing.T) {
 					Initiator:  initiator.String(),
 					Challenger: challenger.String(),
 					Challengee: challengee.String(),
-					Height:     1,
+					Height:     2,
 					Finished:   true,
 				},
 			},
@@ -73,16 +73,87 @@ func TestMsgServerReportPoPResult(t *testing.T) {
 				Challenge: &types.Challenge{
 					Challenger: challenger.String(),
 					Challengee: challengee.String(),
-					Height:     1,
+					Height:     3,
 					Success:    true,
 					Finished:   true,
 				},
 			},
 			"Initiator is not set: invalid challenge",
 		},
+		{
+			"Invalid pop data",
+			types.MsgReportPopResult{
+				Creator: challenger.String(),
+				Challenge: &types.Challenge{
+					Initiator:  initiator.String(),
+					Challenger: challenger.String(),
+					Challengee: challengee.String(),
+					Height:     4,
+					Success:    true,
+					Finished:   true,
+				},
+			},
+			"PoP report data does not match challenge: invalid challenge",
+		},
+		{
+			"Invalid pop data",
+			types.MsgReportPopResult{
+				Creator: challenger.String(),
+				Challenge: &types.Challenge{
+					Initiator:  initiator.String(),
+					Challenger: challenger.String(),
+					Challengee: challengee.String(),
+					Height:     5,
+					Success:    false,
+					Finished:   false,
+				},
+			},
+			"PoP reporter is not the challenger: invalid PoP reporter",
+		},
+		{
+			"Invalid pop data",
+			types.MsgReportPopResult{
+				Creator: challenger.String(),
+				Challenge: &types.Challenge{
+					Initiator:  initiator.String(),
+					Challenger: challenger.String(),
+					Challengee: challengee.String(),
+					Height:     6,
+					Success:    true,
+					Finished:   true,
+				},
+			},
+			"PoP report data does not match challenge: invalid challenge",
+		},
+		{
+			"Non-Existing PoP",
+			types.MsgReportPopResult{
+				Creator: challenger.String(),
+				Challenge: &types.Challenge{
+					Initiator:  initiator.String(),
+					Challenger: challenger.String(),
+					Challengee: challengee.String(),
+					Height:     7,
+					Success:    true,
+					Finished:   true,
+				},
+			},
+			"no challenge found for PoP report: invalid challenge",
+		},
 	}
 
-	msgServer, ctx := setupMsgServer(t)
+	msgServer, ctx, k := setupMsgServer(t)
+	sdkCtx := sdk.UnwrapSDKContext(ctx)
+	// set up the challenges, do not store the last challenge (special test case)
+	for i := 0; i < 6; i++ {
+		msg := testCases[i].msg
+		challenge := msg.GetChallenge()
+		k.StoreChallenge(sdkCtx, *challenge)
+	}
+	// adjust challenge 4 to satisfy the test case
+	testCases[3].msg.Challenge.Challengee = testCases[3].msg.Challenge.Challenger
+	testCases[4].msg.Challenge.Challenger = testCases[4].msg.Challenge.Challengee
+	testCases[5].msg.Challenge.Initiator = testCases[5].msg.Challenge.Challenger
 
 	for _, tc := range testCases {
 		res, err := msgServer.ReportPopResult(ctx, &tc.msg)
@@ -101,7 +172,7 @@ func TestMsgServerMintToken(t *testing.T) {
 	mintRequest := sample.MintRequest(beneficiary, 1000, "hash")
 
 	msg := types.NewMsgMintToken(minter, &mintRequest)
-	msgServer, ctx := setupMsgServer(t)
+	msgServer, ctx, _ := setupMsgServer(t)
 	res, err := msgServer.MintToken(ctx, msg)
 	if assert.NoError(t, err) {
 		assert.Equal(t, &types.MsgMintTokenResponse{}, res)
@@ -121,7 +192,7 @@ func TestMsgServerMintTokenInvalidAddress(t *testing.T) {
 	mintRequest := sample.MintRequest(beneficiary, 1000, "hash")
 
 	msg := types.NewMsgMintToken(minter, &mintRequest)
-	msgServer, ctx := setupMsgServer(t)
+	msgServer, ctx, _ := setupMsgServer(t)
 	_, err := msgServer.MintToken(ctx, msg)
 	if assert.Error(t, err) {
 		assert.EqualError(t, err, fmt.Sprintf("for provided address %s: invalid address", beneficiary))

x/dao/types/errors.go

@@ -28,4 +28,5 @@ var (
 	ErrDistributionWrongHeight  = errorsmod.Register(ModuleName, 19, "distribution wrong height")
 	ErrConvertClaims            = errorsmod.Register(ModuleName, 20, "convert claim failed")
 	ErrInvalidClaimAddress      = errorsmod.Register(ModuleName, 21, "invalid claim address")
+	ErrInvalidPopReporter       = errorsmod.Register(ModuleName, 22, "invalid PoP reporter")
 )