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

Check for deleted accounts in crypto allowances #8834

Merged
merged 11 commits into from
Oct 3, 2023
Merged

Check for deleted accounts in crypto allowances #8834

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3cf5a82
Check for deleted accounts in crypto allowances
mhess-swl Sep 22, 2023
bb9cb0b
Add missing 'final'
mhess-swl Sep 22, 2023
606f631
Use existing validation method
mhess-swl Sep 22, 2023
8f46885
Fix more delete allowance suite tests
mhess-swl Sep 22, 2023
bdc1ed6
Merge branch 'develop' into 08465_effective-owner
mhess-swl Sep 22, 2023
b37fddc
rename variable
mhess-swl Sep 22, 2023
50fcd90
Don't add redundant payer to keys
mhess-swl Sep 22, 2023
cf0ace6
Fix aggregation of nft delete allowances
mhess-swl Sep 22, 2023
bdec6d8
Fix broken approve allowance tests
mhess-swl Sep 22, 2023
230df98
Merge branch 'develop' into 08465_effective-owner
mhess-swl Sep 25, 2023
4963338
Merge branch 'develop' into 08465_effective-owner
mhess-swl Oct 2, 2023
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
Prev Previous commit
Next Next commit
Fix aggregation of nft delete allowances 
Signed-off-by: Matt Hess <matt.hess@swirldslabs.com>
  • Loading branch information
@mhess-swl
mhess-swl committed Sep 22, 2023
commit cf0ace62b6d0f4d554b0b96446066b07956ea750
1 change: 0 additions & 1 deletion ...in/java/com/hedera/node/app/service/token/impl/handlers/CryptoDeleteAllowanceHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx
pureChecks(txn);
final var op = txn.cryptoDeleteAllowanceOrThrow();
// Every owner whose allowances are being removed should sign (or the payer, if there is no owner)
final var payerId = txn.transactionID().accountID();
for (final var allowance : op.nftAllowancesOrElse(emptyList())) {
if (allowance.hasOwner()) {
context.requireKeyOrThrow(allowance.ownerOrThrow(), INVALID_ALLOWANCE_OWNER_ID);
Expand Down
6 changes: 1 addition & 5 deletions ...main/java/com/hedera/node/app/service/token/impl/validators/DeleteAllowanceValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import com.hedera.node.app.spi.workflows.HandleContext;
import com.hedera.node.config.data.HederaConfig;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.util.HashSet;
import java.util.List;
import javax.inject.Inject;
import javax.inject.Singleton;
Expand Down Expand Up @@ -148,11 +147,8 @@ private void validateAllowancesCount(
*/
public static int aggregateNftDeleteAllowances(final List<NftRemoveAllowance> nftAllowances) {
var count = 0;
final var serialsSet = new HashSet<Long>();
for (final var allowance : nftAllowances) {
serialsSet.addAll(allowance.serialNumbers());
count += serialsSet.size();
serialsSet.clear();
count += allowance.serialNumbers().size();
}
return count;
}
Expand Down
15 changes: 6 additions & 9 deletions ...ients/src/main/java/com/hedera/services/bdd/suites/crypto/CryptoDeleteAllowanceSuite.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INVALID_ALLOWANCE_OWNER_ID;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INVALID_TOKEN_NFT_SERIAL_NUMBER;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.MAX_ALLOWANCES_EXCEEDED;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.OK;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.SENDER_DOES_NOT_OWN_NFT_SERIAL_NO;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.TOKEN_NOT_ASSOCIATED_TO_ACCOUNT;
import static com.hederahashgraph.api.proto.java.TokenType.NON_FUNGIBLE_UNIQUE;
Expand Down Expand Up @@ -509,30 +508,28 @@ private HapiSpec exceedsTransactionLimit() {
List.of(
ByteString.copyFromUtf8("a"),
ByteString.copyFromUtf8("b"),
ByteString.copyFromUtf8("c"),
ByteString.copyFromUtf8("d"),
ByteString.copyFromUtf8("e")))
ByteString.copyFromUtf8("c")))
.via("nftTokenMint"),
mintToken(token, 500L).via("tokenMint"),
cryptoTransfer(movingUnique(nft, 1L, 2L, 3L, 4L, 5L).between(TOKEN_TREASURY, owner)))
cryptoTransfer(movingUnique(nft, 1L, 2L, 3L).between(TOKEN_TREASURY, owner)))
.when(
cryptoApproveAllowance()
.payingWith(owner)
.addNftAllowance(owner, nft, spender, false, List.of(1L, 2L)),
cryptoDeleteAllowance()
.payingWith(owner)
.addNftDeleteAllowance(owner, nft, List.of(1L, 1L, 1L, 1L, 1L))
.hasPrecheck(OK),
.addNftDeleteAllowance(owner, nft, List.of(1L, 2L, 3L, 3L, 3L))
.hasPrecheck(MAX_ALLOWANCES_EXCEEDED),
cryptoDeleteAllowance()
.payingWith(owner)
.addNftDeleteAllowance(owner, nft, List.of(1L, 2L, 3L, 4L, 5L))
.addNftDeleteAllowance(owner, nft, List.of(1L, 1L, 1L, 1L, 1L))
.hasPrecheck(MAX_ALLOWANCES_EXCEEDED),
cryptoDeleteAllowance()
.payingWith(owner)
.addNftDeleteAllowance(owner, nft, List.of(1L))
.addNftDeleteAllowance(owner, nft, List.of(2L))
.addNftDeleteAllowance(owner, nft, List.of(3L))
.addNftDeleteAllowance(owner, nft, List.of(4L))
.addNftDeleteAllowance(owner, nft, List.of(1L))
.addNftDeleteAllowance(owner, nft, List.of(1L))
.hasPrecheck(MAX_ALLOWANCES_EXCEEDED))
.then(
Expand Down