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

Jump to bottom

Retain existing allowance (spender) columns when updating NFT metadata (0.112) #9119

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Retain existing allowance (spender) columns when updating NFT metadata (0.112) #9119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 13 additions & 0 deletions hedera-mirror-common/src/main/java/com/hedera/mirror/common/domain/entity/EntityId.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ public final class EntityId implements Serializable, Comparable<EntityId> {

public static final EntityId EMPTY = new EntityId(0L);

/*
* Indicates a domain entity ID is not being set (to null or non-null) in the current operation and that
* the existing DB column value is to be preserved. This is reflected as the value of -1 that can be
* referenced within the @UpsertColumn syntax. See the AbstractNft delegatingSpender and spender
* fields as examples. In this case, this sentinel value is set in TokenUpdateNftsTransactionHandler.
*/
public static final EntityId UNSET = new EntityId();

static final int NUM_BITS = 32;
static final int REALM_BITS = 16;
static final int SHARD_BITS = 15;
Expand Down Expand Up @@ -77,6 +85,11 @@ private EntityId(long id) {
this.id = id;
}

// Used only to construct constant UNSET above
private EntityId() {
this.id = -1L;
}

/**
* Encodes given shard, realm, num into an 8 bytes long.
* <p/>
Expand Down
4 changes: 2 additions & 2 deletions hedera-mirror-common/src/main/java/com/hedera/mirror/common/domain/token/AbstractNft.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public abstract class AbstractNft implements History {
@Column(updatable = false)
private Long createdTimestamp;

@UpsertColumn(coalesce = "{0}")
@UpsertColumn(coalesce = "case when delegating_spender < 0 then e_{0} else {0} end")
private EntityId delegatingSpender;

private Boolean deleted;
Expand All @@ -58,7 +58,7 @@ public abstract class AbstractNft implements History {
@jakarta.persistence.Id
private long serialNumber;

@UpsertColumn(coalesce = "{0}")
@UpsertColumn(coalesce = "case when spender < 0 then e_{0} else {0} end")
private EntityId spender;

@jakarta.persistence.Id
Expand Down
11 changes: 11 additions & 0 deletions .../src/main/java/com/hedera/mirror/importer/parser/record/entity/sql/SqlEntityListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,17 @@ private Nft mergeNft(Nft cachedNft, Nft newNft) {
src.setTimestampUpper(dest.getTimestampLower());
}

/*
* An unset spender field indicates this is an NFT metadata only update, and that the existing allowance
* information must be retained from the source.
*/
if (EntityId.UNSET.equals(dest.getSpender())) {
dest.setSpender(src.getSpender());
}
if (EntityId.UNSET.equals(dest.getDelegatingSpender())) {
dest.setDelegatingSpender(src.getDelegatingSpender());
}

return dest;
}

Expand Down
2 changes: 2 additions & 0 deletions ...a/mirror/importer/parser/record/transactionhandler/TokenUpdateNftsTransactionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ protected void doUpdateTransaction(Transaction transaction, RecordItem recordIte
var consensusTimestamp = recordItem.getConsensusTimestamp();
var nftBuilder = Nft.builder()
.metadata(toBytes(transactionBody.getMetadata().getValue()))
.delegatingSpender(EntityId.UNSET)
.spender(EntityId.UNSET)
.timestampRange(Range.atLeast(consensusTimestamp))
.tokenId(tokenId.getId());

Expand Down
102 changes: 102 additions & 0 deletions ...va/com/hedera/mirror/importer/parser/record/entity/EntityRecordItemListenerTokenTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Range;
import com.google.common.collect.Streams;
import com.google.protobuf.BytesValue;
import com.google.protobuf.StringValue;
import com.hedera.mirror.common.domain.contract.ContractLog;
import com.hedera.mirror.common.domain.contract.ContractResult;
import com.hedera.mirror.common.domain.entity.Entity;
import com.hedera.mirror.common.domain.entity.EntityId;
import com.hedera.mirror.common.domain.entity.EntityTransaction;
import com.hedera.mirror.common.domain.token.AbstractNft;
import com.hedera.mirror.common.domain.token.AbstractNft.Id;
import com.hedera.mirror.common.domain.token.CustomFee;
import com.hedera.mirror.common.domain.token.Nft;
Expand Down Expand Up @@ -3340,6 +3342,106 @@ void tokenWipeNftMissingNft() {
assertThat(findHistory(Nft.class)).isEmpty();
}

@ParameterizedTest
@ValueSource(booleans = {false, true})
void tokenCreateNftCollectionAssociateAllowanceUpdateMetadata(boolean singleRecordFile) {
// given
createAndAssociateToken(
TOKEN_ID,
NON_FUNGIBLE_UNIQUE,
SYMBOL,
CREATE_TIMESTAMP,
ASSOCIATE_TIMESTAMP,
PAYER2,
false,
false,
false,
TokenFreezeStatusEnum.NOT_APPLICABLE,
TokenKycStatusEnum.NOT_APPLICABLE,
TokenPauseStatusEnum.NOT_APPLICABLE,
0);

// mint
long mintTimestamp = CREATE_TIMESTAMP + 20L;
var metadata = recordItemBuilder.bytes(16);
var mintRecordItem = recordItemBuilder
.tokenMint()
.transactionBody(b -> b.clear().setToken(TOKEN_ID).addMetadata(metadata))
.receipt(r -> r.clearSerialNumbers().addSerialNumbers(1).setNewTotalSupply(1))
.record(r -> r.setConsensusTimestamp(TestUtils.toTimestamp(mintTimestamp))
.addTokenTransferLists(TokenTransferList.newBuilder()
.setToken(TOKEN_ID)
.addNftTransfers(NftTransfer.newBuilder()
.setReceiverAccountID(PAYER)
.setSerialNumber(1))))
.build();

// approve allowance
var approveAllowanceTimestamp = mintTimestamp + 20L;
var approveAllowanceRecordItem = recordItemBuilder
.cryptoApproveAllowance()
.transactionBody(b -> b.clear()
.addNftAllowances(NftAllowance.newBuilder()
.addSerialNumbers(1)
.setSpender(PAYER2)
.setDelegatingSpender(PAYER3)
.setTokenId(TOKEN_ID)))
.transactionBodyWrapper(w -> w.setTransactionID(Utility.getTransactionId(PAYER)))
.record(r -> r.setConsensusTimestamp(TestUtils.toTimestamp(approveAllowanceTimestamp)))
.build();

var updateNftMetadataTimestamp = approveAllowanceTimestamp + 20L;
var newMetadata = BytesValue.of(recordItemBuilder.bytes(16));
var updateNftMetadataRecordItem = recordItemBuilder
.tokenUpdateNfts()
.transactionBody(b -> b.clearSerialNumbers()
.setToken(TOKEN_ID)
.addSerialNumbers(1L)
.setMetadata(newMetadata))
.record(r -> r.setConsensusTimestamp(TestUtils.toTimestamp(updateNftMetadataTimestamp)))
.build();

var recordItems = List.of(mintRecordItem, approveAllowanceRecordItem, updateNftMetadataRecordItem);

// when
if (singleRecordFile) {
parseRecordItemsAndCommit(recordItems);
} else {
recordItems.forEach(this::parseRecordItemAndCommit);
}

// then
assertThat(tokenRepository.findById(DOMAIN_TOKEN_ID.getId()))
.get()
.returns(CREATE_TIMESTAMP, Token::getCreatedTimestamp)
.returns(TokenFreezeStatusEnum.NOT_APPLICABLE, Token::getFreezeStatus)
.returns(TokenKycStatusEnum.NOT_APPLICABLE, Token::getKycStatus)
.returns(TokenPauseStatusEnum.NOT_APPLICABLE, Token::getPauseStatus)
.returns(SYMBOL, Token::getSymbol)
.returns(1L, Token::getTotalSupply);

assertThat(nftRepository.findById(new AbstractNft.Id(1L, DOMAIN_TOKEN_ID.getId())))
.get()
.returns(mintTimestamp, Nft::getCreatedTimestamp)
.returns(DomainUtils.toBytes(newMetadata.getValue()), Nft::getMetadata)
.returns(EntityId.of(PAYER3), Nft::getDelegatingSpender)
.returns(EntityId.of(PAYER2), Nft::getSpender);

var nftHistory = findHistory(Nft.class);
assertThat(nftHistory)
.hasSize(2)
.satisfiesExactly(
// First history row written when allowance created, pre-allowance spender columns are null.
n -> assertThat(n)
.returns(null, Nft::getDelegatingSpender)
.returns(null, Nft::getSpender),
// Second history row written when NFT metadata was updated. Allowance set spender columns must
// be indicated.
n -> assertThat(n)
.returns(EntityId.of(PAYER3), Nft::getDelegatingSpender)
.returns(EntityId.of(PAYER2), Nft::getSpender));
}

@Test
void tokenCreateAndAssociateAndWipeInSameRecordFile() {
long transferAmount = -1000L;
Expand Down
3 changes: 3 additions & 0 deletions ...rror/importer/parser/record/transactionhandler/TokenUpdateNftsTransactionHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static org.mockito.Mockito.verifyNoMoreInteractions;

import com.google.common.collect.Range;
import com.hedera.mirror.common.domain.entity.EntityId;
import com.hedera.mirror.common.domain.entity.EntityType;
import com.hedera.mirror.common.domain.token.AbstractNft;
import com.hedera.mirror.common.domain.token.Nft;
Expand Down Expand Up @@ -84,6 +85,8 @@ void updateNftMetadata() {
.isNotNull()
.returns(expectedMetadata, Nft::getMetadata)
.returns(expectedSerialNumbers.get(i), AbstractNft::getSerialNumber)
.returns(EntityId.UNSET, Nft::getDelegatingSpender)
.returns(EntityId.UNSET, Nft::getSpender)
.returns(Range.atLeast(recordItem.getConsensusTimestamp()), Nft::getTimestampRange)
.returns(transaction.getEntityId().getId(), AbstractNft::getTokenId);
}
Expand Down
Loading