Skip to content

Commit

Permalink
fix: (cherry pick) Enhance purechecks for CryptoCreateHandler (#12797) (
Browse files Browse the repository at this point in the history 
#13142)

Signed-off-by: Thomas Moran <152873392+thomas-swirlds-labs@users.noreply.github.com>
Signed-off-by: Neeharika-Sompalli <neeharika.sompalli@swirldslabs.com>
Co-authored-by: Neeharika-Sompalli <neeharika.sompalli@swirldslabs.com>
  • Loading branch information
@thomas-swirlds-labs @Neeharika-Sompalli
thomas-swirlds-labs and Neeharika-Sompalli authored May 9, 2024
1 parent 60ab9d2 commit c5061c0
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 2 deletions.
16 changes: 15 additions & 1 deletion ...pl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoCreateHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@
import static com.hedera.hapi.node.base.ResponseCodeEnum.ALIAS_ALREADY_ASSIGNED;
import static com.hedera.hapi.node.base.ResponseCodeEnum.BAD_ENCODING;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INSUFFICIENT_PAYER_BALANCE;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_ACCOUNT_ID;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_ALIAS_KEY;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_INITIAL_BALANCE;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_PAYER_ACCOUNT_ID;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_RECEIVE_RECORD_THRESHOLD;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_RENEWAL_PERIOD;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_SEND_RECORD_THRESHOLD;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_TRANSACTION_BODY;
import static com.hedera.hapi.node.base.ResponseCodeEnum.KEY_NOT_PROVIDED;
import static com.hedera.hapi.node.base.ResponseCodeEnum.KEY_REQUIRED;
import static com.hedera.hapi.node.base.ResponseCodeEnum.MAX_ENTITIES_IN_PRICE_REGIME_HAVE_BEEN_CREATED;
Expand Down Expand Up @@ -111,15 +113,27 @@ public CryptoCreateHandler(
@Override
public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException {
final var op = txn.cryptoCreateAccountOrThrow();
validateTruePreCheck(op.initialBalance() >= 0L, INVALID_INITIAL_BALANCE);
// Note: validation lives here for now but should take place in handle in the future
validateTruePreCheck(op.hasAutoRenewPeriod(), INVALID_RENEWAL_PERIOD);
validateTruePreCheck(op.autoRenewPeriodOrThrow().seconds() >= 0, INVALID_RENEWAL_PERIOD);
if (op.hasShardID()) {
validateTruePreCheck(op.shardIDOrThrow().shardNum() == 0, INVALID_ACCOUNT_ID);
}
if (op.hasRealmID()) {
validateTruePreCheck(op.realmIDOrThrow().realmNum() == 0, INVALID_ACCOUNT_ID);
}
// HIP 904 now allows for unlimited auto-associations
validateTruePreCheck(op.maxAutomaticTokenAssociations() >= -1, INVALID_TRANSACTION_BODY);
validateTruePreCheck(op.initialBalance() >= 0L, INVALID_INITIAL_BALANCE);
// FUTURE: should this return SEND_RECORD_THRESHOLD_FIELD_IS_DEPRECATED
validateTruePreCheck(op.sendRecordThreshold() >= 0L, INVALID_SEND_RECORD_THRESHOLD);
// FUTURE: should this return RECEIVE_RECORD_THRESHOLD_FIELD_IS_DEPRECATED
validateTruePreCheck(op.receiveRecordThreshold() >= 0L, INVALID_RECEIVE_RECORD_THRESHOLD);
validateTruePreCheck(
op.proxyAccountIDOrElse(AccountID.DEFAULT).equals(AccountID.DEFAULT),
PROXY_ACCOUNT_ID_FIELD_IS_DEPRECATED);
// sendRecordThreshold, receiveRecordThreshold and proxyAccountID are deprecated. So no need to check them.
validateFalsePreCheck(op.hasProxyAccountID(), PROXY_ACCOUNT_ID_FIELD_IS_DEPRECATED);
// You can never set the alias to be an "entity num alias" (sometimes called "long-zero").
final var alias = op.alias();
validateFalsePreCheck(isEntityNumAlias(alias), INVALID_ALIAS_KEY);
Expand Down
31 changes: 31 additions & 0 deletions ...st/java/com/hedera/node/app/service/token/impl/test/handlers/CryptoCreateHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_RECEIVE_RECORD_THRESHOLD;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_RENEWAL_PERIOD;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_SEND_RECORD_THRESHOLD;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_TRANSACTION_BODY;
import static com.hedera.hapi.node.base.ResponseCodeEnum.KEY_REQUIRED;
import static com.hedera.hapi.node.base.ResponseCodeEnum.MEMO_TOO_LONG;
import static com.hedera.hapi.node.base.ResponseCodeEnum.NOT_SUPPORTED;
Expand Down Expand Up @@ -201,6 +202,21 @@ void whenNoAutoRenewPeriodSpecified() throws PreCheckException {
assertEquals(INVALID_RENEWAL_PERIOD, msg.responseCode());
}

@Test
@DisplayName("pureChecks succeeds when expected shardId is specified")
void validateWhenZeroShardId() throws PreCheckException {
txn = new CryptoCreateBuilder().withShardId(0).build();
assertDoesNotThrow(() -> subject.pureChecks(txn));
}

@Test
@DisplayName("pureChecks fail when invalid maxAutoAssociations is specified")
void failsWhenInvalidMaxAutoAssociations() throws PreCheckException {
txn = new CryptoCreateBuilder().withMaxAutoAssociations(-5).build();
final var msg = assertThrows(PreCheckException.class, () -> subject.pureChecks(txn));
assertEquals(INVALID_TRANSACTION_BODY, msg.responseCode());
}

@Test
@DisplayName("pureChecks fail when negative send record threshold is specified")
void sendRecordThresholdIsNegative() throws PreCheckException {
Expand Down Expand Up @@ -717,6 +733,8 @@ private class CryptoCreateBuilder {
private long receiveRecordThreshold = 0;
private AccountID proxyAccountId = null;
private long stakedAccountId = 0;
private long shardId = 0;
private int maxAutoAssociations = -1;

private Key key = otherKey;

Expand Down Expand Up @@ -754,6 +772,9 @@ public TransactionBody build() {
if (memo != null) {
createTxnBody.memo(memo);
}
if (maxAutoAssociations != -1) {
createTxnBody.maxAutomaticTokenAssociations(maxAutoAssociations);
}

return TransactionBody.newBuilder()
.transactionID(transactionID)
Expand Down Expand Up @@ -821,5 +842,15 @@ public CryptoCreateBuilder withKey(final Key key) {
this.key = key;
return this;
}

public CryptoCreateBuilder withShardId(final long id) {
this.shardId = id;
return this;
}

public CryptoCreateBuilder withMaxAutoAssociations(final int maxAutoAssociations) {
this.maxAutoAssociations = maxAutoAssociations;
return this;
}
}
}
17 changes: 16 additions & 1 deletion ...ents/src/main/java/com/hedera/services/bdd/spec/transactions/crypto/HapiCryptoCreate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
import com.hederahashgraph.api.proto.java.FeeData;
import com.hederahashgraph.api.proto.java.HederaFunctionality;
import com.hederahashgraph.api.proto.java.Key;
import com.hederahashgraph.api.proto.java.RealmID;
import com.hederahashgraph.api.proto.java.ShardID;
import com.hederahashgraph.api.proto.java.TokenID;
import com.hederahashgraph.api.proto.java.Transaction;
import com.hederahashgraph.api.proto.java.TransactionBody;
Expand Down Expand Up @@ -96,6 +98,8 @@ public class HapiCryptoCreate extends HapiTxnOp<HapiCryptoCreate> {
private Consumer<Address> addressObserver;
private boolean fuzzingIdentifiers = false;
private boolean setEvmAddressAliasFromKey = false;
private Optional<ShardID> shardId = Optional.empty();
private Optional<RealmID> realmId = Optional.empty();

@Override
public HederaFunctionality type() {
Expand Down Expand Up @@ -231,6 +235,16 @@ public HapiCryptoCreate alias(final ByteString alias) {
return this;
}

public HapiCryptoCreate shardId(final ShardID shardID) {
this.shardId = Optional.of(shardID);
return this;
}

public HapiCryptoCreate realmId(final RealmID realmID) {
this.realmId = Optional.of(realmID);
return this;
}

public HapiCryptoCreate evmAddress(final ByteString evmAddress) {
this.evmAddress = Optional.of(evmAddress);
return this;
Expand Down Expand Up @@ -293,7 +307,8 @@ protected Consumer<TransactionBody.Builder> opBodyDef(final HapiSpec spec) throw
autoRenewDurationSecs.ifPresent(s -> b.setAutoRenewPeriod(
Duration.newBuilder().setSeconds(s).build()));
maxAutomaticTokenAssociations.ifPresent(b::setMaxAutomaticTokenAssociations);

shardId.ifPresent(b::setShardID);
realmId.ifPresent(b::setRealmID);
if (stakedAccountId.isPresent()) {
b.setStakedAccountId(asId(stakedAccountId.get(), spec));
} else if (stakedNodeId.isPresent()) {
Expand Down
6 changes: 6 additions & 0 deletions ...e/test-clients/src/main/java/com/hedera/services/bdd/suites/crypto/CryptoCreateSuite.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@
import com.hedera.services.bdd.suites.HapiSuite;
import com.hederahashgraph.api.proto.java.Key;
import com.hederahashgraph.api.proto.java.KeyList;
import com.hederahashgraph.api.proto.java.RealmID;
import com.hederahashgraph.api.proto.java.ShardID;
import com.hederahashgraph.api.proto.java.ThresholdKey;
import com.swirlds.common.utility.CommonUtils;
import java.util.List;
Expand Down Expand Up @@ -305,6 +307,8 @@ final HapiSpec createAnAccountEmptyThresholdKey() {
final HapiSpec createAnAccountEmptyKeyList() {
KeyShape shape = listOf(0);
long initialBalance = 10_000L;
ShardID shardID = ShardID.newBuilder().build();
RealmID realmID = RealmID.newBuilder().build();

return defaultHapiSpec("createAnAccountEmptyKeyList")
.given()
Expand All @@ -313,6 +317,8 @@ final HapiSpec createAnAccountEmptyKeyList() {
cryptoCreate(NO_KEYS)
.keyShape(shape)
.balance(initialBalance)
.shardId(shardID)
.realmId(realmID)
.logged()
.hasPrecheck(KEY_REQUIRED)
// In modular code this error is thrown in handle, but it is fixed using dynamic property
Expand Down

0 comments on commit c5061c0

Please sign in to comment.