Skip to content

Commit

Permalink
Fix nft owner mapping from nft readable state (#10488)
Browse files Browse the repository at this point in the history 
This PR fixes nft owner mapping returned from readable kv states:
Modularized services expects the owner of the nft to be null when the owner is the token treasury and return the owner id when the owner is different than the token treasury.

This can be verified by the burn nft logic and nft docs:

private boolean treasuryOwnsNft(final AccountID ownerID) {
        return ownerID == null;
}


* @param ownerId <b>(2)</b> The account or contract id that owns this NFT.
 *                <p>
 *                If this NFT is owned by its token type's current treasury account,
 *                this value SHALL be zero.

In order to make it work according to the docs the following change is applied:
NftReadableKVState - add tokenRepository to be able to get the treasury of the token.
After obtaining the treasury of the token we verify that the nft owner is different than the token treasury.
If it is different the we return it otherwise we return null as the docs suggests.

NftReadableKVState - adds new checks for token owner and treasury
NftReadableKVStateTest - modify according to new changes

---------

Signed-off-by: Kristiyan Selveliev <[email protected]>
  • Loading branch information
@kselveliev
kselveliev authored Feb 26, 2025
1 parent 8904707 commit 1ffbbb5
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 37 deletions.
35 changes: 31 additions & 4 deletions ...a-mirror-web3/src/main/java/com/hedera/mirror/web3/state/keyvalue/NftReadableKVState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,20 @@

import static com.hedera.mirror.web3.state.Utils.convertToTimestamp;

import com.hedera.hapi.node.base.AccountID;
import com.hedera.hapi.node.base.NftID;
import com.hedera.hapi.node.base.TokenID;
import com.hedera.hapi.node.state.token.Nft;
import com.hedera.mirror.common.domain.entity.EntityId;
import com.hedera.mirror.common.domain.token.AbstractToken;
import com.hedera.mirror.web3.common.ContractCallContext;
import com.hedera.mirror.web3.repository.NftRepository;
import com.hedera.mirror.web3.repository.TokenRepository;
import com.hedera.pbj.runtime.io.buffer.Bytes;
import com.hedera.services.utils.EntityIdUtils;
import jakarta.annotation.Nonnull;
import jakarta.inject.Named;
import java.util.Optional;

/**
* This class serves as a repository layer between hedera app services read only state and the Postgres database in
Expand All @@ -24,10 +29,12 @@ public class NftReadableKVState extends AbstractReadableKVState<NftID, Nft> {

public static final String KEY = "NFTS";
private final NftRepository nftRepository;
private final TokenRepository tokenRepository;

public NftReadableKVState(@Nonnull NftRepository nftRepository) {
public NftReadableKVState(@Nonnull NftRepository nftRepository, @Nonnull TokenRepository tokenRepository) {
super(KEY);
this.nftRepository = nftRepository;
this.tokenRepository = tokenRepository;
}

@Override
Expand All @@ -38,20 +45,40 @@ protected Nft readFromDataSource(@Nonnull final NftID key) {

final var timestamp = ContractCallContext.get().getTimestamp();
final var nftId = EntityIdUtils.toEntityId(key.tokenId()).getId();
final var tokenTreasury = getTokenTreasury(nftId, timestamp);

return timestamp
.map(t -> nftRepository.findActiveByIdAndTimestamp(nftId, key.serialNumber(), t))
.orElseGet(() -> nftRepository.findActiveById(nftId, key.serialNumber()))
.map(nft -> mapToNft(nft, key.tokenId()))
.map(nft -> mapToNft(nft, key.tokenId(), tokenTreasury))
.orElse(null);
}

private Nft mapToNft(final com.hedera.mirror.common.domain.token.Nft nft, final TokenID tokenID) {
private Nft mapToNft(
final com.hedera.mirror.common.domain.token.Nft nft,
final TokenID tokenID,
final EntityId treasuryAccountId) {
return Nft.newBuilder()
.metadata(Bytes.wrap(nft.getMetadata()))
.mintTime(convertToTimestamp(nft.getCreatedTimestamp()))
.nftId(new NftID(tokenID, nft.getSerialNumber()))
.ownerId(EntityIdUtils.toAccountId(nft.getAccountId()))
.ownerId(getOwnerId(nft.getAccountId(), treasuryAccountId))
.spenderId(EntityIdUtils.toAccountId(nft.getSpender()))
.build();
}

private EntityId getTokenTreasury(final long nftId, Optional<Long> timestamp) {
return timestamp
.flatMap(t -> tokenRepository.findByTokenIdAndTimestamp(nftId, t))
.or(() -> tokenRepository.findById(nftId))
.map(AbstractToken::getTreasuryAccountId)
.orElse(null);
}

private AccountID getOwnerId(final EntityId accountId, final EntityId treasuryAccountId) {
if (accountId == null || accountId.equals(treasuryAccountId)) {
return null;
}
return EntityIdUtils.toAccountId(accountId);
}
}
11 changes: 4 additions & 7 deletions ...irror-web3/src/test/java/com/hedera/mirror/web3/service/ContractCallDynamicCallsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,7 @@ void burnTokenGetTotalSupplyAndBalanceOfTreasury(final TokenTypeEnum tokenType)
fungibleTokenPersist(tokenEntity, treasuryAccount);
} else {
var token = nonFungibleTokenPersist(tokenEntity, treasuryAccount);
nonFungibleTokenInstancePersist(
token,
1L,
mirrorNodeEvmProperties.isModularizedServices() ? null : treasuryAccount.toEntityId(),
treasuryAccount.toEntityId());
nonFungibleTokenInstancePersist(token, 1L, treasuryAccount.toEntityId(), treasuryAccount.toEntityId());
}

final var contract = testWeb3jService.deploy(DynamicEthCalls::deploy);
Expand Down Expand Up @@ -165,8 +161,9 @@ void pauseTokenGetPauseStatusUnpauseGetPauseStatus(final TokenTypeEnum tokenType
}

/**
* The test calls HederaTokenService.freezeToken(token, account) precompiled system contract to
* freeze a given fungible/non-fungible token for a given account.
* The test calls HederaTokenService.freezeToken(token, account) precompiled system contract to freeze a given
* fungible/non-fungible token for a given account.
*
* @param tokenType
*/
@ParameterizedTest
Expand Down
4 changes: 2 additions & 2 deletions ...c/test/java/com/hedera/mirror/web3/service/ContractCallServiceERCTokenHistoricalTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ void name(final boolean isStatic) throws Exception {
@ValueSource(booleans = {true, false})
void ownerOf(final boolean isStatic) throws Exception {
// Given
final var owner = accountEntityPersistWithEvmAddressHistorical(historicalRange);
final var owner = accountEntityPersistHistorical(historicalRange);
final var nftToken = nftPersistHistorical(owner.toEntityId());
tokenAccountFrozenRelationshipPersistHistorical(nftToken, owner, historicalRange);
final var contract = testWeb3jService.deploy(ERCTestContractHistorical::deploy);
Expand All @@ -563,7 +563,7 @@ void ownerOf(final boolean isStatic) throws Exception {
: contract.call_getOwnerOfNonStatic(getAddressFromEntity(nftToken), DEFAULT_SERIAL_NUMBER)
.send();
// Then
assertThat(result).isEqualTo(getAliasFromEntity(owner));
assertThat(result).isEqualTo(getAddressFromEntity(owner));
}

@ParameterizedTest
Expand Down
6 changes: 4 additions & 2 deletions ...st/java/com/hedera/mirror/web3/service/ContractCallServicePrecompileModificationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,10 @@ void burnNFT() throws Exception {

tokenBalancePersist(treasury.toEntityId(), EntityId.of(tokenId), treasury.getBalanceTimestamp());

final var nft = nftPersistCustomizable(n -> n.tokenId(tokenId)
.accountId(mirrorNodeEvmProperties.isModularizedServices() ? null : treasury.toEntityId()));
final var nft = domainBuilder
.nft()
.customize(n -> n.tokenId(tokenId).serialNumber(1L).accountId(treasury.toEntityId()))
.persist();

domainBuilder
.nftHistory()
Expand Down
82 changes: 60 additions & 22 deletions ...rror-web3/src/test/java/com/hedera/mirror/web3/state/keyvalue/NftReadableKVStateTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.hedera.mirror.common.domain.token.Nft;
import com.hedera.mirror.web3.common.ContractCallContext;
import com.hedera.mirror.web3.repository.NftRepository;
import com.hedera.mirror.web3.repository.TokenRepository;
import com.hedera.pbj.runtime.io.buffer.Bytes;
import com.hedera.services.utils.EntityIdUtils;
import java.util.Collections;
Expand All @@ -38,6 +39,7 @@ class NftReadableKVStateTest {
TokenID.newBuilder().shardNum(0L).realmNum(0L).tokenNum(1252L).build();
private static final Optional<Long> timestamp = Optional.of(1726231985623004672L);
private static final EntityId spender = EntityId.of(1L, 2L, 3L);
private static final EntityId treasury = EntityId.of(1L, 2L, 4L);
private static final NftID NFT_ID = new NftID(TOKEN_ID, 1L);
private static MockedStatic<ContractCallContext> contextMockedStatic;

Expand All @@ -47,6 +49,9 @@ class NftReadableKVStateTest {
@Mock
private NftRepository nftRepository;

@Mock
private TokenRepository tokenRepository;

private DomainBuilder domainBuilder;
private Entity entity;

Expand Down Expand Up @@ -93,6 +98,24 @@ void getNftMappedValuesWithTimestamp() {
.returns(null, com.hedera.hapi.node.state.token.Nft::ownerNextNftId));
}

@Test
void getNftMappedValuesWithTreasuryAsOwner() {
when(contractCallContext.getTimestamp()).thenReturn(timestamp);
Nft nftDomain = setupNftWithOwner(timestamp);
assertThat(nftReadableKVState.readFromDataSource(NFT_ID)).satisfies(nft -> assertThat(nft)
.returns(NFT_ID, com.hedera.hapi.node.state.token.Nft::nftId)
.returns(null, com.hedera.hapi.node.state.token.Nft::ownerId)
.returns(
EntityIdUtils.toAccountId(nftDomain.getSpender()),
com.hedera.hapi.node.state.token.Nft::spenderId)
.returns(
convertToTimestamp(nftDomain.getCreatedTimestamp()),
com.hedera.hapi.node.state.token.Nft::mintTime)
.returns(Bytes.wrap(nftDomain.getMetadata()), com.hedera.hapi.node.state.token.Nft::metadata)
.returns(null, com.hedera.hapi.node.state.token.Nft::ownerPreviousNftId)
.returns(null, com.hedera.hapi.node.state.token.Nft::ownerNextNftId));
}

@Test
void getNftMappedValuesWithoutTimestamp() {
when(contractCallContext.getTimestamp()).thenReturn(Optional.empty());
Expand Down Expand Up @@ -171,39 +194,54 @@ void testSize() {
assertThat(nftReadableKVState.size()).isZero();
}

private Nft setupNft(Optional<Long> timestamp) {
Nft databaseNft = domainBuilder
private Nft setupNft(Optional<Long> timestamp, final EntityId spender, final EntityId owner) {
final var databaseNft = domainBuilder
.nft()
.customize(t -> t.tokenId(entity.getId())
.serialNumber(NFT_ID.serialNumber())
.spender(spender))
.customize(t -> {
t.tokenId(entity.getId()).serialNumber(NFT_ID.serialNumber());
if (spender != null) {
t.spender(spender);
}
if (owner != null) {
t.accountId(owner);
}
})
.get();

final var databaseToken = domainBuilder
.token()
.customize(t -> {
t.tokenId(entity.getId()).treasuryAccountId(treasury);
if (owner != null) {
t.treasuryAccountId(owner);
}
})
.get();

if (timestamp.isPresent()) {
databaseNft.setCreatedTimestamp(timestamp.get());
when(nftRepository.findActiveByIdAndTimestamp(entity.getId(), NFT_ID.serialNumber(), timestamp.get()))
long ts = timestamp.get();
databaseNft.setCreatedTimestamp(ts);
when(tokenRepository.findByTokenIdAndTimestamp(entity.getId(), ts)).thenReturn(Optional.of(databaseToken));
when(nftRepository.findActiveByIdAndTimestamp(entity.getId(), NFT_ID.serialNumber(), ts))
.thenReturn(Optional.of(databaseNft));
} else {
when(tokenRepository.findById(entity.getId())).thenReturn(Optional.of(databaseToken));
when(nftRepository.findActiveById(entity.getId(), NFT_ID.serialNumber()))
.thenReturn(Optional.ofNullable(databaseNft));
.thenReturn(Optional.of(databaseNft));
}

return databaseNft;
}

private Nft setupNftMissingSpender(Optional<Long> timestamp) {
Nft databaseNft = domainBuilder
.nft()
.customize(t -> t.tokenId(entity.getId()).serialNumber(NFT_ID.serialNumber()))
.get();
private Nft setupNft(Optional<Long> timestamp) {
return setupNft(timestamp, spender, null);
}

if (timestamp.isPresent()) {
databaseNft.setCreatedTimestamp(timestamp.get());
when(nftRepository.findActiveByIdAndTimestamp(entity.getId(), NFT_ID.serialNumber(), timestamp.get()))
.thenReturn(Optional.of(databaseNft));
} else {
when(nftRepository.findActiveById(entity.getId(), NFT_ID.serialNumber()))
.thenReturn(Optional.ofNullable(databaseNft));
}
return databaseNft;
private Nft setupNftWithOwner(Optional<Long> timestamp) {
return setupNft(timestamp, null, treasury);
}

private Nft setupNftMissingSpender(Optional<Long> timestamp) {
return setupNft(timestamp, null, null);
}
}

0 comments on commit 1ffbbb5

Please sign in to comment.