Skip to content

Commit

Permalink
fix: ERC approve and remove scenarios (#10325)
Browse files Browse the repository at this point in the history 
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
Co-authored-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
Co-authored-by: Michael Tinker <michael.tinker@swirldslabs.com>
  • Loading branch information
@mustafauzunn @stoyanov-st @tinker-michaelj
3 people committed Dec 21, 2023
1 parent 65116fe commit f10afe6
Show file tree
Hide file tree
Showing 10 changed files with 247 additions and 12 deletions.
82 changes: 73 additions & 9 deletions ...rvice/contract/impl/exec/systemcontracts/hts/grantapproval/AbstractGrantApprovalCall.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,16 @@

package com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.grantapproval;

import static java.util.Objects.requireNonNull;

import com.hedera.hapi.node.base.AccountID;
import com.hedera.hapi.node.base.TokenID;
import com.hedera.hapi.node.base.TokenType;
import com.hedera.hapi.node.state.token.AccountApprovalForAllAllowance;
import com.hedera.hapi.node.token.CryptoApproveAllowanceTransactionBody;
import com.hedera.hapi.node.token.CryptoDeleteAllowanceTransactionBody;
import com.hedera.hapi.node.token.NftAllowance;
import com.hedera.hapi.node.token.NftRemoveAllowance;
import com.hedera.hapi.node.token.TokenAllowance;
import com.hedera.hapi.node.transaction.TransactionBody;
import com.hedera.node.app.service.contract.impl.exec.gas.SystemContractGasCalculator;
Expand All @@ -29,6 +34,7 @@
import com.hedera.node.app.service.contract.impl.hevm.HederaWorldUpdater.Enhancement;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.math.BigInteger;
import java.util.List;

public abstract class AbstractGrantApprovalCall extends AbstractHtsCall {
protected final VerificationStrategy verificationStrategy;
Expand Down Expand Up @@ -60,32 +66,90 @@ protected AbstractGrantApprovalCall(
}

public TransactionBody callGrantApproval() {
return TransactionBody.newBuilder()
.cryptoApproveAllowance(approve(token, spender, amount, tokenType))
if (tokenType == TokenType.NON_FUNGIBLE_UNIQUE) {
var ownerId = getOwnerId();

if (ownerId != null && !isNftApprovalRevocation()) {
List<AccountApprovalForAllAllowance> accountApprovalForAllAllowances = enhancement
.nativeOperations()
.getAccount(ownerId.accountNum())
.approveForAllNftAllowances();
if (accountApprovalForAllAllowances != null) {
for (var approvedForAll : accountApprovalForAllAllowances) {
if (approvedForAll.tokenId().equals(token)) {
return buildCryptoApproveAllowance(approveDelegate(ownerId, approvedForAll.spenderId()));
}
}
}
}

return isNftApprovalRevocation()
? buildCryptoDeleteAllowance(remove(ownerId))
: buildCryptoApproveAllowance(approve(ownerId));
} else {
return buildCryptoApproveAllowance(approve(senderId));
}
}

private CryptoDeleteAllowanceTransactionBody remove(AccountID ownerId) {
return CryptoDeleteAllowanceTransactionBody.newBuilder()
.nftAllowances(NftRemoveAllowance.newBuilder()
.tokenId(token)
.owner(ownerId)
.serialNumbers(amount.longValue())
.build())
.build();
}

private CryptoApproveAllowanceTransactionBody approve(
@NonNull final TokenID token,
@NonNull final AccountID spender,
@NonNull final BigInteger amount,
@NonNull final TokenType tokenType) {
private CryptoApproveAllowanceTransactionBody approveDelegate(AccountID ownerId, AccountID delegateSpenderId) {
return CryptoApproveAllowanceTransactionBody.newBuilder()
.nftAllowances(NftAllowance.newBuilder()
.tokenId(token)
.spender(spender)
.delegatingSpender(delegateSpenderId)
.owner(ownerId)
.serialNumbers(amount.longValue())
.build())
.build();
}

private CryptoApproveAllowanceTransactionBody approve(AccountID ownerId) {
return tokenType.equals(TokenType.FUNGIBLE_COMMON)
? CryptoApproveAllowanceTransactionBody.newBuilder()
.tokenAllowances(TokenAllowance.newBuilder()
.tokenId(token)
.spender(spender)
.owner(senderId)
.owner(ownerId)
.amount(amount.longValue())
.build())
.build()
: CryptoApproveAllowanceTransactionBody.newBuilder()
.nftAllowances(NftAllowance.newBuilder()
.tokenId(token)
.spender(spender)
.owner(senderId)
.owner(ownerId)
.serialNumbers(amount.longValue())
.build())
.build();
}

private TransactionBody buildCryptoDeleteAllowance(CryptoDeleteAllowanceTransactionBody body) {
return TransactionBody.newBuilder().cryptoDeleteAllowance(body).build();
}

private TransactionBody buildCryptoApproveAllowance(CryptoApproveAllowanceTransactionBody body) {
return TransactionBody.newBuilder().cryptoApproveAllowance(body).build();
}

private AccountID getOwnerId() {
final var nft = enhancement.nativeOperations().getNft(token.tokenNum(), amount.longValue());
requireNonNull(nft);
return nft.hasOwnerId()
? nft.ownerId()
: enhancement.nativeOperations().getToken(token.tokenNum()).treasuryAccountId();
}

private boolean isNftApprovalRevocation() {
return spender.accountNum() == 0;
}
}
23 changes: 23 additions & 0 deletions ...pp/service/contract/impl/exec/systemcontracts/hts/grantapproval/ERCGrantApprovalCall.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@

package com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.grantapproval;

import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_ALLOWANCE_SPENDER_ID;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_TOKEN_ID;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.FullResult.revertResult;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.FullResult.successResult;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.HtsSystemContract.HTS_EVM_ADDRESS;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.HtsCall.PricedResult.gasOnly;
import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.asEvmContractId;
import static com.hedera.node.app.service.contract.impl.utils.SystemContractUtils.contractFunctionResultFailedForProto;

import com.hedera.hapi.node.base.AccountID;
import com.hedera.hapi.node.base.ResponseCodeEnum;
Expand All @@ -28,10 +32,14 @@
import com.hedera.node.app.service.contract.impl.exec.gas.DispatchType;
import com.hedera.node.app.service.contract.impl.exec.gas.SystemContractGasCalculator;
import com.hedera.node.app.service.contract.impl.exec.scope.VerificationStrategy;
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.FullResult;
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.ReturnTypes;
import com.hedera.node.app.service.contract.impl.hevm.HederaWorldUpdater.Enhancement;
import com.hedera.node.app.spi.workflows.record.SingleTransactionRecordBuilder;
import com.hedera.pbj.runtime.io.buffer.Bytes;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.math.BigInteger;
import org.hyperledger.besu.datatypes.Address;

public class ERCGrantApprovalCall extends AbstractGrantApprovalCall {

Expand All @@ -53,7 +61,22 @@ public PricedResult execute() {
if (token == null) {
return reversionWith(INVALID_TOKEN_ID, gasCalculator.canonicalGasRequirement(DispatchType.APPROVE));
}
final var spenderAccount = enhancement.nativeOperations().getAccount(spender.accountNum());
final var body = callGrantApproval();
if (spenderAccount == null && spender.accountNum() != 0) {
var gasRequirement = gasCalculator.canonicalGasRequirement(DispatchType.APPROVE);
var revertResult = FullResult.revertResult(INVALID_ALLOWANCE_SPENDER_ID, gasRequirement);
var result = gasOnly(revertResult, INVALID_ALLOWANCE_SPENDER_ID, false);

var contractID = asEvmContractId(Address.fromHexString(HTS_EVM_ADDRESS));
var encodedRc = ReturnTypes.encodedRc(INVALID_ALLOWANCE_SPENDER_ID).array();
var contractFunctionResult = contractFunctionResultFailedForProto(
gasRequirement, INVALID_ALLOWANCE_SPENDER_ID.protoName(), contractID, Bytes.wrap(encodedRc));

enhancement.systemOperations().externalizeResult(contractFunctionResult, INVALID_ALLOWANCE_SPENDER_ID);

return result;
}
final var recordBuilder = systemContractOperations()
.dispatch(body, verificationStrategy, senderId, SingleTransactionRecordBuilder.class);
final var gasRequirement = gasCalculator.gasRequirement(body, DispatchType.APPROVE, senderId);
Expand Down
23 changes: 23 additions & 0 deletions ...pl/src/main/java/com/hedera/node/app/service/contract/impl/utils/SystemContractUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,29 @@ public static ContractFunctionResult contractFunctionResultFailedFor(
.build();
}

/**
* Create an error contract function result.
*
* @param gasUsed Report the gas used.
* @param errorMsg The error message to report back to the caller.
* @param contractID The contract ID.
* @param contractCallResult Bytes representation of the contract call result error
* @return The created contract function result when for a failed call.
*/
@NonNull
public static ContractFunctionResult contractFunctionResultFailedForProto(
final long gasUsed,
final String errorMsg,
final ContractID contractID,
final com.hedera.pbj.runtime.io.buffer.Bytes contractCallResult) {
return ContractFunctionResult.newBuilder()
.gasUsed(gasUsed)
.contractID(contractID)
.errorMessage(errorMsg)
.contractCallResult(contractCallResult)
.build();
}

private static ContractID contractIdFromEvmAddress(final byte[] bytes) {
return ContractID.newBuilder()
.contractNum(Longs.fromByteArray(Arrays.copyOfRange(bytes, 12, 20)))
Expand Down
2 changes: 2 additions & 0 deletions ...ervice-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/TestHelpers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,8 @@ public class TestHelpers {
public static final Address OWNER_BESU_ADDRESS = pbjToBesuAddress(OWNER_ADDRESS);
public static final AccountID UNAUTHORIZED_SPENDER_ID =
AccountID.newBuilder().accountNum(999999L).build();
public static final AccountID REVOKE_APPROVAL_SPENDER_ID =
AccountID.newBuilder().accountNum(0L).build();
public static final Bytes UNAUTHORIZED_SPENDER_ADDRESS = Bytes.fromHex("b284224b8b83a724438cc3cc7c0d333a2b6b3222");
public static final com.esaulpaugh.headlong.abi.Address UNAUTHORIZED_SPENDER_HEADLONG_ADDRESS =
asHeadlongAddress(UNAUTHORIZED_SPENDER_ADDRESS.toByteArray());
Expand Down
16 changes: 16 additions & 0 deletions ...ntract/impl/test/exec/systemcontracts/hts/grantapproval/ClassicGrantApprovalCallTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@

import com.hedera.hapi.node.base.ResponseCodeEnum;
import com.hedera.hapi.node.base.TokenType;
import com.hedera.hapi.node.state.token.Account;
import com.hedera.hapi.node.state.token.Nft;
import com.hedera.hapi.node.state.token.Token;
import com.hedera.node.app.service.contract.impl.exec.gas.SystemContractGasCalculator;
import com.hedera.node.app.service.contract.impl.exec.scope.VerificationStrategy;
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.grantapproval.ClassicGrantApprovalCall;
Expand All @@ -51,6 +54,15 @@ public class ClassicGrantApprovalCallTest extends HtsCallTestBase {
@Mock
private SystemContractGasCalculator systemContractGasCalculator;

@Mock
private Nft nft;

@Mock
private Token token;

@Mock
private Account account;

@Test
void fungibleApprove() {
subject = new ClassicGrantApprovalCall(
Expand Down Expand Up @@ -87,6 +99,10 @@ void nftApprove() {
TokenType.NON_FUNGIBLE_UNIQUE);
given(systemContractOperations.dispatch(any(), any(), any(), any())).willReturn(recordBuilder);
given(recordBuilder.status()).willReturn(ResponseCodeEnum.SUCCESS);
given(nativeOperations.getNft(NON_FUNGIBLE_TOKEN_ID.tokenNum(), 100L)).willReturn(nft);
given(nativeOperations.getToken(NON_FUNGIBLE_TOKEN_ID.tokenNum())).willReturn(token);
given(token.treasuryAccountId()).willReturn(OWNER_ID);
given(nativeOperations.getAccount(OWNER_ID.accountNum())).willReturn(account);
final var result = subject.execute(frame).fullResult().result();

assertEquals(MessageFrame.State.COMPLETED_SUCCESS, result.getState());
Expand Down
78 changes: 78 additions & 0 deletions ...e/contract/impl/test/exec/systemcontracts/hts/grantapproval/ERCGrantApprovalCallTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,20 @@
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.FUNGIBLE_TOKEN_ID;
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.NON_FUNGIBLE_TOKEN_ID;
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.OWNER_ID;
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.REVOKE_APPROVAL_SPENDER_ID;
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.UNAUTHORIZED_SPENDER_ID;
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.asBytesResult;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;

import com.hedera.hapi.node.base.ResponseCodeEnum;
import com.hedera.hapi.node.base.TokenType;
import com.hedera.hapi.node.state.token.Account;
import com.hedera.hapi.node.state.token.Nft;
import com.hedera.hapi.node.state.token.Token;
import com.hedera.hapi.node.transaction.TransactionBody;
import com.hedera.node.app.service.contract.impl.exec.gas.SystemContractGasCalculator;
import com.hedera.node.app.service.contract.impl.exec.scope.VerificationStrategy;
Expand All @@ -37,7 +42,9 @@
import com.hedera.node.app.service.token.records.CryptoTransferRecordBuilder;
import com.hedera.node.app.spi.workflows.record.SingleTransactionRecordBuilder;
import java.math.BigInteger;
import org.apache.tuweni.bytes.Bytes;
import org.hyperledger.besu.evm.frame.MessageFrame;
import org.hyperledger.besu.evm.frame.MessageFrame.State;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;

Expand All @@ -54,6 +61,15 @@ class ERCGrantApprovalCallTest extends HtsCallTestBase {
@Mock
private CryptoTransferRecordBuilder recordBuilder;

@Mock
private Nft nft;

@Mock
private Token token;

@Mock
private Account account;

@Test
void erc20approve() {
subject = new ERCGrantApprovalCall(
Expand All @@ -72,6 +88,7 @@ void erc20approve() {
eq(SingleTransactionRecordBuilder.class)))
.willReturn(recordBuilder);
given(recordBuilder.status()).willReturn(ResponseCodeEnum.SUCCESS);
given(nativeOperations.getAccount(anyLong())).willReturn(account);
final var result = subject.execute().fullResult().result();

assertEquals(MessageFrame.State.COMPLETED_SUCCESS, result.getState());
Expand Down Expand Up @@ -99,6 +116,67 @@ void erc721approve() {
eq(SingleTransactionRecordBuilder.class)))
.willReturn(recordBuilder);
given(recordBuilder.status()).willReturn(ResponseCodeEnum.SUCCESS);
given(nativeOperations.getNft(NON_FUNGIBLE_TOKEN_ID.tokenNum(), 100L)).willReturn(nft);
given(nativeOperations.getToken(NON_FUNGIBLE_TOKEN_ID.tokenNum())).willReturn(token);
given(token.treasuryAccountId()).willReturn(OWNER_ID);
given(nativeOperations.getAccount(anyLong())).willReturn(account);
final var result = subject.execute().fullResult().result();

assertEquals(MessageFrame.State.COMPLETED_SUCCESS, result.getState());
assertEquals(
asBytesResult(GrantApprovalTranslator.ERC_GRANT_APPROVAL_NFT
.getOutputs()
.encodeElements()),
result.getOutput());
}

@Test
void erc721approveFailsWithInvalidSpenderAllowance() {
subject = new ERCGrantApprovalCall(
mockEnhancement(),
systemContractGasCalculator,
verificationStrategy,
OWNER_ID,
NON_FUNGIBLE_TOKEN_ID,
UNAUTHORIZED_SPENDER_ID,
BigInteger.valueOf(100L),
TokenType.NON_FUNGIBLE_UNIQUE);
given(nativeOperations.getNft(NON_FUNGIBLE_TOKEN_ID.tokenNum(), 100L)).willReturn(nft);
given(nativeOperations.getToken(NON_FUNGIBLE_TOKEN_ID.tokenNum())).willReturn(token);
given(token.treasuryAccountId()).willReturn(OWNER_ID);
given(nativeOperations.getAccount(anyLong())).willReturn(null).willReturn(account);
final var result = subject.execute().fullResult().result();

assertEquals(State.REVERT, result.getState());
assertEquals(
Bytes.wrap(ResponseCodeEnum.INVALID_ALLOWANCE_SPENDER_ID
.protoName()
.getBytes()),
result.getOutput());
}

@Test
void erc721revoke() {
subject = new ERCGrantApprovalCall(
mockEnhancement(),
systemContractGasCalculator,
verificationStrategy,
OWNER_ID,
NON_FUNGIBLE_TOKEN_ID,
REVOKE_APPROVAL_SPENDER_ID,
BigInteger.valueOf(100L),
TokenType.NON_FUNGIBLE_UNIQUE);
given(systemContractOperations.dispatch(
any(TransactionBody.class),
eq(verificationStrategy),
eq(OWNER_ID),
eq(SingleTransactionRecordBuilder.class)))
.willReturn(recordBuilder);
given(recordBuilder.status()).willReturn(ResponseCodeEnum.SUCCESS);
given(nativeOperations.getNft(NON_FUNGIBLE_TOKEN_ID.tokenNum(), 100L)).willReturn(nft);
given(nativeOperations.getToken(NON_FUNGIBLE_TOKEN_ID.tokenNum())).willReturn(token);
given(nativeOperations.getAccount(anyLong())).willReturn(account);
given(token.treasuryAccountId()).willReturn(OWNER_ID);
final var result = subject.execute().fullResult().result();

assertEquals(MessageFrame.State.COMPLETED_SUCCESS, result.getState());
Expand Down
Loading

0 comments on commit f10afe6

Please sign in to comment.