thetatoken / theta-eth-rpc-adaptor

An adaptor that translates the Theta RPC APIs to the Ethereum RPC APIs
https://docs.thetatoken.org/
24 stars 11 forks source link

Incorrect events emitted by the Theta blockchain #26

Closed rjx18 closed 2 years ago

rjx18 commented 2 years ago

Hi there! This is Richard from TKETS again. I have observed a rather strange thing happening on the blockchain explorer and the RPC endpoint, and was wondering why this is happening.

First of all, for our ticket smart contract, we have a mint ticket function as follows (for your reference):

function mintTicket(uint256 numberOfTickets) external payable {
        # checks if the number of tickets is not 0
        require(numberOfTickets != 0, TICKET_SALE_ERROR);

        # sets the commission and adds to the price of the ticket
        uint256 ticketCommission = SafeMath.div(SafeMath.mul(ticketPrice, factory.commissionRate()), 10000);
        uint256 ticketSalePrice = SafeMath.add(ticketPrice, ticketCommission);

        # calculates the expected TFUEL value paid for this mint transaction, based on the number of tickets
        uint256 ticketSaleValue = SafeMath.mul(ticketSalePrice, numberOfTickets);

        # check if the message amount is equal or greater than (in case of donations) the total ticket sale value calculated previously
        require(msg.value == ticketSaleValue || (metadata.acceptDonations && msg.value > ticketSaleValue), TICKET_SALE_ERROR);

        # check if event exists in the factory
        require(!factory.eventToStatus(eventId), TICKET_SALE_ERROR);

        # check if minting is within ticket sale time
        require(block.timestamp > SafeMath.sub(metadata.ticketStartTime, 10) && block.timestamp < metadata.ticketEndTime, TICKET_SALE_ERROR); // have some leeway for starting time

        # get current total supply of tickets
        uint256 currentTicketCount = totalSupply();

        # add the number of tickets to be minted
        uint256 endTicketCount = SafeMath.add(currentTicketCount, numberOfTickets);

        # check if this end ticket count is not greater than the maximum tickets allowed
        require(metadata.maxTickets == 0 || endTicketCount <= metadata.maxTickets, TICKET_SALE_ERROR);

        # for each ticket in number of tickets minted, mint the ticket to the sender address
        for (uint256 i = 1; i <= numberOfTickets; i++) {
            # adds i to the current ticket count and emits a mint event
            uint256 nextTokenId = currentTicketCount + i;
            super._mint(msg.sender, nextTokenId);
            emit TicketMint(msg.sender, nextTokenId);
        }

        # pay ticket commission if there is
        if (ticketCommission > 0) {
            payable(factory).transfer(SafeMath.mul(ticketCommission, numberOfTickets));
        }
    }

I've checked this many times and I don't see how in any case, the same token ID could be minted twice. There is no possibility of reentrancy here, because the only external contract we call is our own EventFactory contract. Yet, take a look at the following sequence of transactions:

Transaction 1

Explorer link: https://explorer.thetatoken.org/txs/0x8c1266aec59c934e1ae8798f3623623e9a8684d367cced22c8404260dd632e50 Event 0x38724e6ad4034b018042d9630894fc289eef8ca90c53de83ce512efd5e638bba topics (our mint event): Topic 1: 0x00000000000000000000000080ca09c8622f62ec83fb32049adbd007aff51571 (the owner address) Topic 2: 0x000000000000000000000000000000000000000000000000000000000000028f (the token ID minted, in hexadecimal)

This is fine for now, until we look at the next two transactions.

Transaction 2

Explorer link: https://explorer.thetatoken.org/txs/0x21a1f5872991c7e14977a89456a449742b438dbdc8346423bf4dc249479b7d53 Event 0x38724e6ad4034b018042d9630894fc289eef8ca90c53de83ce512efd5e638bba topics (our mint event): Topic 1: 0x000000000000000000000000cf46d7def9a16966661506ecd9f2235cfa166377 (the owner address) Topic 2: 0x0000000000000000000000000000000000000000000000000000000000000295 (the token ID minted, in hexadecimal)

This should be 290, because that is the next succeeding token ID. But if we look at the next transaction...

Transaction 3

Explorer link: https://explorer.thetatoken.org/txs/0x4ed593597e64bb60aae6b035586eccc0c2fb719481288fa3f2ce8102bd0f37e3 Event 0x38724e6ad4034b018042d9630894fc289eef8ca90c53de83ce512efd5e638bba topics (our mint event): Topic 1: 0x0000000000000000000000007003053a96979b76ffd7359aa288bd583d74807c (the owner address) Topic 2: 0x0000000000000000000000000000000000000000000000000000000000000291 (the token ID minted, in hexadecimal)

... (there are multiple events for this transaction)

Event 0x38724e6ad4034b018042d9630894fc289eef8ca90c53de83ce512efd5e638bba topics (our mint event): Topic 1: 0x0000000000000000000000007003053a96979b76ffd7359aa288bd583d74807c (the owner address) Topic 2: 0x0000000000000000000000000000000000000000000000000000000000000295 (the token ID minted, in hexadecimal)

Here we see that the first ticket minted is 291, which is correct. However, this person mints 5 tickets in total, and therefore mints ticket 295 again, which should not be possible.

If we query the smart contract directly for the owner of the token ID 656 (which is in hexadecimal 290, our missing ticket) using the following script run in truffle:

const TNT721 = artifacts.require("TNT721");

module.exports = async function(callback) {
  // perform actions
  const NFTInstance = await TNT721.at("0x0478578c5e906afeb1bdbbf358929affbf1575c8");
  let ownerOfToken = await NFTInstance.ownerOf("656")
  console.log(ownerOfToken)
  callback()
}

we get the following owner: 0xcf46d7DeF9a16966661506EcD9f2235Cfa166377, which is the same owner of the ticket minted in transaction 2. Therefore, this means that our code is functional, because the event we emit uses the same nextTokenId as the one we call the _mint() function with. If there was a bug in the minting loop, both the event and the owner stored on the blockchain would be wrong. I suspect that there may be some problem with the logs emitted for this particular block, or perhaps just some issues with caching on the Theta Explorer and RPC endpoint? I'm not sure. This is the only occurence of this particular problem for us. Could you advice on what might be wrong here? Thank you!

jieyilong commented 2 years ago

Thanks for reporting this. I don't think we do caching in the RPC endpoint. We'd need to take a deeper look to understand why this might happen.

jieyilong commented 2 years ago

This should have been fixed now with this pull request. Let us know if you are still seeing this issue.

https://github.com/thetatoken/theta-protocol-ledger/pull/178