Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use owner event to populate ERC721 transfer topic #1932

Merged
merged 3 commits into from
Nov 20, 2024
Merged

Use owner event to populate ERC721 transfer topic #1932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
72320ac
Use owner event to populate ERC721 transfer topic
codchen Nov 14, 2024
42d3acf
more test
codchen Nov 14, 2024
72f99e5
rebase and revert owner replacement
codchen Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions app/receipt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
// 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)
log, eligible := app.translateCW721Event(wasmToEvmEventCtx, wasmEvent, pointerAddr, contractAddr, response)
if eligible {
log.Index = uint(len(logs))
logs = append(logs, log)
Expand Down Expand Up @@ -173,7 +173,7 @@
return nil, false
}

func (app *App) translateCW721Event(ctx sdk.Context, wasmEvent abci.Event, pointerAddr common.Address, contractAddr string) (*ethtypes.Log, bool) {
func (app *App) translateCW721Event(ctx sdk.Context, wasmEvent abci.Event, pointerAddr common.Address, contractAddr string, response sdk.DeliverTxHookInput) (*ethtypes.Log, bool) {
action, found := GetAttributeValue(wasmEvent, "action")
if !found {
return nil, false
Expand All @@ -185,9 +185,35 @@
if tokenID == nil {
return nil, false
}
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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be good to add a comment here re. why we're doing this - something like
"ERC721 standards dictate that the "sender" in the logs are the actual owner, and in cases where the owner is not the caller, we look for a matching token_id and extract the owner from synthetic events"

if len(ownerEvent.Attributes) != 3 ||
string(ownerEvent.Attributes[0].Key) != wasmtypes.AttributeKeyContractAddr ||
string(ownerEvent.Attributes[0].Value) != contractAddr {
continue

Check warning on line 199 in app/receipt.go

View check run for this annotation

Codecov / codecov/patch

app/receipt.go#L199 

Added line #L199 was not covered by tests
}
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

Check warning on line 205 in app/receipt.go

View check run for this annotation

Codecov / codecov/patch

app/receipt.go#L205 

Added line #L205 was not covered by tests
}
ownerAcc, err := sdk.AccAddressFromBech32(string(ownerEvent.Attributes[2].Value))
if err != nil {
continue

Check warning on line 209 in app/receipt.go

View check run for this annotation

Codecov / codecov/patch

app/receipt.go#L209 

Added line #L209 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we log the error? (albeit rare/unlikely)

}
owner := app.EvmKeeper.GetEVMAddressOrDefault(ctx, ownerAcc)
sender = common.BytesToHash(owner[:])
}
topics = []common.Hash{
ERC721TransferTopic,
app.GetEvmAddressAttribute(ctx, wasmEvent, "sender"),
sender,
app.GetEvmAddressAttribute(ctx, wasmEvent, "recipient"),
common.BigToHash(tokenID),
}
Expand Down
75 changes: 65 additions & 10 deletions app/receipt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ func TestEvmEventsForCw721(t *testing.T) {
amt := sdk.NewCoins(sdk.NewCoin("usei", sdk.NewInt(1000000000000)))
k.BankKeeper().MintCoins(ctx, "evm", amt)
k.BankKeeper().SendCoinsFromModuleToAccount(ctx, "evm", creator, amt)
recipient, _ := testkeeper.MockAddressPair()
privKeyRecipient := testkeeper.MockPrivateKey()
recipient, _ := testkeeper.PrivateKeyToAddresses(privKeyRecipient)
payload := []byte(fmt.Sprintf("{\"mint\":{\"token_id\":\"1\",\"owner\":\"%s\"}}", recipient.String()))
msg := &wasmtypes.MsgExecuteContract{
Sender: creator.String(),
Expand Down Expand Up @@ -320,18 +321,20 @@ func TestEvmEventsForCw721(t *testing.T) {
require.True(t, found)
require.Equal(t, common.HexToHash("0x1").Bytes(), receipt.Logs[0].Data)

// revoke all
payload = []byte(fmt.Sprintf("{\"revoke_all\":{\"operator\":\"%s\"}}", recipient.String()))
// transfer on behalf
k.BankKeeper().MintCoins(ctx, "evm", amt)
k.BankKeeper().SendCoinsFromModuleToAccount(ctx, "evm", recipient, amt)
payload = []byte(fmt.Sprintf("{\"transfer_nft\":{\"token_id\":\"2\",\"recipient\":\"%s\"}}", recipient.String()))
msg = &wasmtypes.MsgExecuteContract{
Sender: creator.String(),
Sender: recipient.String(),
Contract: contractAddr.String(),
Msg: payload,
}
txBuilder = testkeeper.EVMTestApp.GetTxConfig().NewTxBuilder()
txBuilder.SetMsgs(msg)
txBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin("usei", sdk.NewInt(1000000))))
txBuilder.SetGasLimit(300000)
tx = signTx(txBuilder, privKey, k.AccountKeeper().GetAccount(ctx, creator))
tx = signTx(txBuilder, privKeyRecipient, k.AccountKeeper().GetAccount(ctx, recipient))
txbz, err = testkeeper.EVMTestApp.GetTxConfig().TxEncoder()(tx)
require.Nil(t, err)
sum = sha256.Sum256(txbz)
Expand All @@ -342,12 +345,66 @@ func TestEvmEventsForCw721(t *testing.T) {
require.Equal(t, 1, len(receipt.Logs))
require.NotEmpty(t, receipt.LogsBloom)
require.Equal(t, mockPointerAddr.Hex(), receipt.Logs[0].Address)
_, found = testkeeper.EVMTestApp.EvmKeeper.GetEVMTxDeferredInfo(ctx)
require.True(t, found)
require.Equal(t, uint32(0), receipt.Logs[0].Index)
ownerHash := receipt.Logs[0].Topics[1]
// make sure that the owner is set correctly to the creator, not the spender.
creatorEvmAddr := testkeeper.EVMTestApp.EvmKeeper.GetEVMAddressOrDefault(ctx, creator)
require.Equal(t, common.BytesToHash(creatorEvmAddr[:]).Hex(), ownerHash)
tokenIdHash = receipt.Logs[0].Topics[3]
require.Equal(t, "0x0000000000000000000000000000000000000000000000000000000000000002", tokenIdHash)
require.Equal(t, common.HexToHash("0x0").Bytes(), receipt.Logs[0].Data)

// burn
// transfer back
payload = []byte(fmt.Sprintf("{\"transfer_nft\":{\"token_id\":\"2\",\"recipient\":\"%s\"}}", creator.String()))
msg = &wasmtypes.MsgExecuteContract{
Sender: recipient.String(),
Contract: contractAddr.String(),
Msg: payload,
}
txBuilder = testkeeper.EVMTestApp.GetTxConfig().NewTxBuilder()
txBuilder.SetMsgs(msg)
txBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin("usei", sdk.NewInt(1000000))))
txBuilder.SetGasLimit(300000)
tx = signTx(txBuilder, privKeyRecipient, k.AccountKeeper().GetAccount(ctx, recipient))
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)

// burn on behalf
payload = []byte("{\"burn\":{\"token_id\":\"2\"}}")
msg = &wasmtypes.MsgExecuteContract{
Sender: recipient.String(),
Contract: contractAddr.String(),
Msg: payload,
}
txBuilder = testkeeper.EVMTestApp.GetTxConfig().NewTxBuilder()
txBuilder.SetMsgs(msg)
txBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin("usei", sdk.NewInt(1000000))))
txBuilder.SetGasLimit(300000)
tx = signTx(txBuilder, privKeyRecipient, k.AccountKeeper().GetAccount(ctx, recipient))
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, 1, len(receipt.Logs))
require.NotEmpty(t, receipt.LogsBloom)
require.Equal(t, mockPointerAddr.Hex(), receipt.Logs[0].Address)
require.Equal(t, uint32(0), receipt.Logs[0].Index)
ownerHash = receipt.Logs[0].Topics[1]
// make sure that the owner is set correctly to the creator, not the spender.
creatorEvmAddr = testkeeper.EVMTestApp.EvmKeeper.GetEVMAddressOrDefault(ctx, creator)
require.Equal(t, common.BytesToHash(creatorEvmAddr[:]).Hex(), ownerHash)
tokenIdHash = receipt.Logs[0].Topics[3]
require.Equal(t, "0x0000000000000000000000000000000000000000000000000000000000000002", tokenIdHash)
require.Equal(t, common.HexToHash("0x0").Bytes(), receipt.Logs[0].Data)

// revoke all
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we also test a transfer on behalf of someone else?

payload = []byte(fmt.Sprintf("{\"revoke_all\":{\"operator\":\"%s\"}}", recipient.String()))
msg = &wasmtypes.MsgExecuteContract{
Sender: creator.String(),
Contract: contractAddr.String(),
Expand All @@ -370,8 +427,6 @@ func TestEvmEventsForCw721(t *testing.T) {
require.Equal(t, mockPointerAddr.Hex(), receipt.Logs[0].Address)
_, found = testkeeper.EVMTestApp.EvmKeeper.GetEVMTxDeferredInfo(ctx)
require.True(t, found)
tokenIdHash = receipt.Logs[0].Topics[3]
require.Equal(t, "0x0000000000000000000000000000000000000000000000000000000000000002", tokenIdHash)
require.Equal(t, common.HexToHash("0x0").Bytes(), receipt.Logs[0].Data)
}

Expand Down
Loading