Assign owner correctly when there are multiple transfers (#1956)

Author: codchen

app/receipt.go

@@ -32,6 +32,10 @@ type AllowanceResponse struct {
 	Expires   json.RawMessage `json:"expires"`
 }
 
+func getOwnerEventKey(contractAddr string, tokenID string) string {
+	return fmt.Sprintf("%s-%s", contractAddr, tokenID)
+}
+
 func (app *App) AddCosmosEventsToEVMReceiptIfApplicable(ctx sdk.Context, tx sdk.Tx, checksum [32]byte, response sdk.DeliverTxHookInput) {
 	// hooks will only be called if DeliverTx is successful
 	wasmEvents := GetEventsOfType(response, wasmtypes.WasmModuleEventType)
@@ -43,6 +47,26 @@ func (app *App) AddCosmosEventsToEVMReceiptIfApplicable(ctx sdk.Context, tx sdk.
 	// additional gas consumption from EVM receipt generation and event translation
 	wasmToEvmEventGasLimit := app.EvmKeeper.GetDeliverTxHookWasmGasLimit(ctx.WithGasMeter(sdk.NewInfiniteGasMeter(1, 1)))
 	wasmToEvmEventCtx := ctx.WithGasMeter(sdk.NewGasMeterWithMultiplier(ctx, wasmToEvmEventGasLimit))
+	// unfortunately CW721 transfer events differ from ERC721 transfer events
+	// in that CW721 include sender (which can be different than owner) whereas
+	// ERC721 always include owner. The following logic refer to the owner
+	// event emitted before the transfer and use that instead to populate the
+	// synthetic ERC721 event.
+	ownerEvents := GetEventsOfType(response, wasmtypes.EventTypeCW721PreTransferOwner)
+	ownerEventsMap := map[string][]abci.Event{}
+	for _, ownerEvent := range ownerEvents {
+		if len(ownerEvent.Attributes) != 3 {
+			ctx.Logger().Error("received owner event with number of attributes != 3")
+			continue
+		}
+		ownerEventKey := getOwnerEventKey(string(ownerEvent.Attributes[0].Value), string(ownerEvent.Attributes[1].Value))
+		if events, ok := ownerEventsMap[ownerEventKey]; ok {
+			ownerEventsMap[ownerEventKey] = append(events, ownerEvent)
+		} else {
+			ownerEventsMap[ownerEventKey] = []abci.Event{ownerEvent}
+		}
+	}
+	cw721TransferCounterMap := map[string]int{}
 	for _, wasmEvent := range wasmEvents {
 		contractAddr, found := GetAttributeValue(wasmEvent, wasmtypes.AttributeKeyContractAddr)
 		if !found {
@@ -60,7 +84,7 @@ func (app *App) AddCosmosEventsToEVMReceiptIfApplicable(ctx sdk.Context, tx sdk.
 		// check if there is a ERC721 pointer to contract Addr
 		pointerAddr, _, exists = app.EvmKeeper.GetERC721CW721Pointer(wasmToEvmEventCtx, contractAddr)
 		if exists {
-			log, eligible := app.translateCW721Event(wasmToEvmEventCtx, wasmEvent, pointerAddr, contractAddr, response)
+			log, eligible := app.translateCW721Event(wasmToEvmEventCtx, wasmEvent, pointerAddr, contractAddr, ownerEventsMap, cw721TransferCounterMap)
 			if eligible {
 				log.Index = uint(len(logs))
 				logs = append(logs, log)
@@ -173,7 +197,8 @@ func (app *App) translateCW20Event(ctx sdk.Context, wasmEvent abci.Event, pointe
 	return nil, false
 }
 
-func (app *App) translateCW721Event(ctx sdk.Context, wasmEvent abci.Event, pointerAddr common.Address, contractAddr string, response sdk.DeliverTxHookInput) (*ethtypes.Log, bool) {
+func (app *App) translateCW721Event(ctx sdk.Context, wasmEvent abci.Event, pointerAddr common.Address, contractAddr string,
+	ownerEventsMap map[string][]abci.Event, cw721TransferCounterMap map[string]int) (*ethtypes.Log, bool) {
 	action, found := GetAttributeValue(wasmEvent, "action")
 	if !found {
 		return nil, false
@@ -184,32 +209,30 @@ func (app *App) translateCW721Event(ctx sdk.Context, wasmEvent abci.Event, point
 		tokenID := GetTokenIDAttribute(wasmEvent)
 		if tokenID == nil {
 			return nil, false
+		} else {
+			ctx.Logger().Error("Translate CW721 error: null token ID")
 		}
-		sender := common.Hash{}
-		// unfortunately CW721 transfer events differ from ERC721 transfer events
-		// in that CW721 include sender (which can be different than owner) whereas
-		// ERC721 always include owner. The following logic refer to the owner
-		// event emitted before the transfer and use that instead to populate the
-		// synthetic ERC721 event.
-		ownerEvents := GetEventsOfType(response, wasmtypes.EventTypeCW721PreTransferOwner)
-		for _, ownerEvent := range ownerEvents {
-			if len(ownerEvent.Attributes) != 3 ||
-				string(ownerEvent.Attributes[0].Key) != wasmtypes.AttributeKeyContractAddr ||
-				string(ownerEvent.Attributes[0].Value) != contractAddr {
-				continue
-			}
-			tokenIDStr, _ := GetAttributeValue(wasmEvent, "token_id")
-			if string(ownerEvent.Attributes[1].Key) != wasmtypes.AttributeKeyTokenId ||
-				string(ownerEvent.Attributes[1].Value) != tokenIDStr ||
-				string(ownerEvent.Attributes[2].Key) != wasmtypes.AttributeKeyOwner {
-				continue
-			}
-			ownerAcc, err := sdk.AccAddressFromBech32(string(ownerEvent.Attributes[2].Value))
-			if err != nil {
-				continue
+		sender := app.GetEvmAddressAttribute(ctx, wasmEvent, "sender")
+		ownerEventKey := getOwnerEventKey(contractAddr, tokenID.String())
+		var currentCounter int
+		if c, ok := cw721TransferCounterMap[ownerEventKey]; ok {
+			currentCounter = c
+		}
+		cw721TransferCounterMap[ownerEventKey] = currentCounter + 1
+		if ownerEvents, ok := ownerEventsMap[ownerEventKey]; ok {
+			if len(ownerEvents) > currentCounter {
+				ownerSeiAddrStr := string(ownerEvents[currentCounter].Attributes[2].Value)
+				if ownerSeiAddr, err := sdk.AccAddressFromBech32(ownerSeiAddrStr); err == nil {
+					ownerEvmAddr := app.EvmKeeper.GetEVMAddressOrDefault(ctx, ownerSeiAddr)
+					sender = common.BytesToHash(ownerEvmAddr[:])
+				} else {
+					ctx.Logger().Error("Translate CW721 error: invalid bech32 owner", "error", err, "address", ownerSeiAddrStr)
+				}
+			} else {
+				ctx.Logger().Error("Translate CW721 error: insufficient owner events", "key", ownerEventKey, "counter", currentCounter, "events", len(ownerEvents))
 			}
-			owner := app.EvmKeeper.GetEVMAddressOrDefault(ctx, ownerAcc)
-			sender = common.BytesToHash(owner[:])
+		} else {
+			ctx.Logger().Error("Translate CW721 error: owner event not found", "key", ownerEventKey)
 		}
 		topics = []common.Hash{
 			ERC721TransferTopic,

app/receipt_test.go

@@ -372,6 +372,41 @@ func TestEvmEventsForCw721(t *testing.T) {
 	res = testkeeper.EVMTestApp.DeliverTx(ctx.WithEventManager(sdk.NewEventManager()), abci.RequestDeliverTx{Tx: txbz}, tx, sum)
 	require.Equal(t, uint32(0), res.Code)
 
+	// acct2 transfer on behalf of acct1 to acct2, acct2 approve acct1, acct1 transfer on behalf of acct2 to acct1
+	txBuilder = testkeeper.EVMTestApp.GetTxConfig().NewTxBuilder()
+	msg1 := &wasmtypes.MsgExecuteContract{
+		Sender:   recipient.String(),
+		Contract: contractAddr.String(),
+		Msg:      []byte(fmt.Sprintf("{\"transfer_nft\":{\"token_id\":\"2\",\"recipient\":\"%s\"}}", recipient.String())),
+	}
+	msg2 := &wasmtypes.MsgExecuteContract{
+		Sender:   recipient.String(),
+		Contract: contractAddr.String(),
+		Msg:      []byte(fmt.Sprintf("{\"approve_all\":{\"operator\":\"%s\"}}", creator.String())),
+	}
+	msg3 := &wasmtypes.MsgExecuteContract{
+		Sender:   creator.String(),
+		Contract: contractAddr.String(),
+		Msg:      []byte(fmt.Sprintf("{\"transfer_nft\":{\"token_id\":\"2\",\"recipient\":\"%s\"}}", creator.String())),
+	}
+	txBuilder.SetMsgs(msg1, msg2, msg3)
+	txBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin("usei", sdk.NewInt(1000000))))
+	txBuilder.SetGasLimit(1000000)
+	tx = signTxMultiple(txBuilder, []cryptotypes.PrivKey{privKeyRecipient, privKey}, []authtypes.AccountI{k.AccountKeeper().GetAccount(ctx, recipient), k.AccountKeeper().GetAccount(ctx, creator)})
+	txbz, err = testkeeper.EVMTestApp.GetTxConfig().TxEncoder()(tx)
+	require.Nil(t, err)
+	sum = sha256.Sum256(txbz)
+	res = testkeeper.EVMTestApp.DeliverTx(ctx.WithEventManager(sdk.NewEventManager()), abci.RequestDeliverTx{Tx: txbz}, tx, sum)
+	require.Equal(t, uint32(0), res.Code)
+	receipt, err = testkeeper.EVMTestApp.EvmKeeper.GetTransientReceipt(ctx, common.BytesToHash(sum[:]))
+	require.Nil(t, err)
+	require.Equal(t, 3, len(receipt.Logs))
+	// make sure that the owner is set correctly to the creator, not the spender.
+	require.Equal(t, common.BytesToHash(creatorEvmAddr[:]).Hex(), receipt.Logs[0].Topics[1])
+	// the second log is the approval log, which doesn't affect ownership thus not checking here.
+	recipientEvmAddr := testkeeper.EVMTestApp.EvmKeeper.GetEVMAddressOrDefault(ctx, recipient)
+	require.Equal(t, common.BytesToHash(recipientEvmAddr[:]).Hex(), receipt.Logs[2].Topics[1])
+
 	// burn on behalf
 	payload = []byte("{\"burn\":{\"token_id\":\"2\"}}")
 	msg = &wasmtypes.MsgExecuteContract{
@@ -460,3 +495,37 @@ func signTx(txBuilder client.TxBuilder, privKey cryptotypes.PrivKey, acc authtyp
 	_ = txBuilder.SetSignatures(sigsV2...)
 	return txBuilder.GetTx()
 }
+
+func signTxMultiple(txBuilder client.TxBuilder, privKeys []cryptotypes.PrivKey, accs []authtypes.AccountI) sdk.Tx {
+	var sigsV2 []signing.SignatureV2
+	for i, privKey := range privKeys {
+		sigsV2 = append(sigsV2, signing.SignatureV2{
+			PubKey: privKey.PubKey(),
+			Data: &signing.SingleSignatureData{
+				SignMode:  testkeeper.EVMTestApp.GetTxConfig().SignModeHandler().DefaultMode(),
+				Signature: nil,
+			},
+			Sequence: accs[i].GetSequence(),
+		})
+	}
+	_ = txBuilder.SetSignatures(sigsV2...)
+	sigsV2 = []signing.SignatureV2{}
+	for i, privKey := range privKeys {
+		signerData := xauthsigning.SignerData{
+			ChainID:       "sei-test",
+			AccountNumber: accs[i].GetAccountNumber(),
+			Sequence:      accs[i].GetSequence(),
+		}
+		sigV2, _ := clienttx.SignWithPrivKey(
+			testkeeper.EVMTestApp.GetTxConfig().SignModeHandler().DefaultMode(),
+			signerData,
+			txBuilder,
+			privKey,
+			testkeeper.EVMTestApp.GetTxConfig(),
+			accs[i].GetSequence(),
+		)
+		sigsV2 = append(sigsV2, sigV2)
+	}
+	_ = txBuilder.SetSignatures(sigsV2...)
+	return txBuilder.GetTx()
+}