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

Waive new key's signature when a privileged payer updates a system account #1164

Merged
merged 8 commits into from
Mar 12, 2021
Merged

Waive new key's signature when a privileged payer updates a system account #1164

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2092eec
wave siging if its a special account and updating a system account
anighanta Mar 11, 2021
b1a7b27
add unit tests
anighanta Mar 11, 2021
d85f11b
bug fix
anighanta Mar 11, 2021
d7e7cf2
add EET suite
anighanta Mar 12, 2021
e15b223
address review comments
anighanta Mar 12, 2021
dc004bc
update docuemntation
anighanta Mar 12, 2021
e4b8b64
undo commented out code for testing
anighanta Mar 12, 2021
08a90d0
Merge branch 'master' into 01148-M-cryptoUpdate-waive-signing
anighanta Mar 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 5 additions & 6 deletions hedera-node/src/main/java/com/hedera/services/sigs/order/HederaSigningOrder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,12 +541,11 @@ private <T> SigningOrderResult<T> cryptoUpdate(
} else if (targetMustSign) {
required = mutable(required);
required.add(result.metadata().getKey());
}

if (op.hasKey()) {
required = mutable(required);
var candidate = asUsableFcKey(op.getKey());
candidate.ifPresent(required::add);
if (op.hasKey()) {
required = mutable(required);
anighanta marked this conversation as resolved.
Show resolved Hide resolved
var candidate = asUsableFcKey(op.getKey());
candidate.ifPresent(required::add);
}
}

return factory.forValidOrder(required);
Expand Down
38 changes: 35 additions & 3 deletions hedera-node/src/test/java/com/hedera/services/sigs/order/HederaSigningOrderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,9 +392,24 @@ public void getsCryptoUpdateProtectedNewKey() throws Throwable {
SigningOrderResult<SignatureStatus> summary = subject.keysForOtherParties(txn, summaryFactory);

// then:
assertThat(
sanityRestored(summary.getOrderedKeys()),
contains(NEW_ACCOUNT_KT.asKey()));
assertTrue(sanityRestored(summary.getOrderedKeys()).isEmpty());
verify(updateSigReqs).test(txn);
}

@Test
public void getsCryptoUpdateProtectedSysAccountNewKey() throws Throwable {
// given:
@SuppressWarnings("unchecked")
Predicate<TransactionBody> updateSigReqs = (Predicate<TransactionBody>)mock(Predicate.class);
setupFor(CRYPTO_UPDATE_SYS_ACCOUNT_WITH_NEW_KEY_SCENARIO, updateSigReqs);
// and:
given(updateSigReqs.test(txn)).willReturn(false);

// when:
SigningOrderResult<SignatureStatus> summary = subject.keysForOtherParties(txn, summaryFactory);

// then:
assertTrue(sanityRestored(summary.getOrderedKeys()).isEmpty());
verify(updateSigReqs).test(txn);
}

Expand All @@ -415,6 +430,23 @@ public void getsCryptoUpdateProtectedNoNewKey() throws Throwable {
verify(updateSigReqs).test(txn);
}

@Test
public void getsCryptoUpdateProtectedSysAccountNoNewKey() throws Throwable {
// given:
@SuppressWarnings("unchecked")
Predicate<TransactionBody> updateSigReqs = (Predicate<TransactionBody>)mock(Predicate.class);
setupFor(CRYPTO_UPDATE_SYS_ACCOUNT_WITH_NO_NEW_KEY_SCENARIO, updateSigReqs);
// and:
given(updateSigReqs.test(txn)).willReturn(false);

// when:
SigningOrderResult<SignatureStatus> summary = subject.keysForOtherParties(txn, summaryFactory);

// then:
assertTrue(sanityRestored(summary.getOrderedKeys()).isEmpty());
verify(updateSigReqs).test(txn);
}

@Test
public void getsCryptoUpdateVanillaNoNewKey() throws Throwable {
// given:
Expand Down
16 changes: 16 additions & 0 deletions hedera-node/src/test/java/com/hedera/test/factories/scenarios/CryptoUpdateScenarios.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,21 @@ public PlatformTxnAccessor platformTxn() throws Throwable {
.get()
));
}
},
CRYPTO_UPDATE_SYS_ACCOUNT_WITH_NEW_KEY_SCENARIO {
public PlatformTxnAccessor platformTxn() throws Throwable {
return new PlatformTxnAccessor(from(
newSignedCryptoUpdate(SYS_ACCOUNT_ID)
.newAccountKt(SYS_ACCOUNT_KT)
.get()
));
}
},
CRYPTO_UPDATE_SYS_ACCOUNT_WITH_NO_NEW_KEY_SCENARIO {
public PlatformTxnAccessor platformTxn() throws Throwable {
return new PlatformTxnAccessor(from(
newSignedCryptoUpdate(SYS_ACCOUNT_ID).get()
));
}
}
}
4 changes: 4 additions & 0 deletions hedera-node/src/test/java/com/hedera/test/factories/scenarios/TxnHandlingScenario.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,9 @@ default ScheduleStore scheduleStore() {
AccountID MISC_ACCOUNT = asAccount(MISC_ACCOUNT_ID);
KeyTree MISC_ACCOUNT_KT = withRoot(ed25519());

String SYS_ACCOUNT_ID = "0.0.666";
AccountID SYS_ACCOUNT = asAccount(SYS_ACCOUNT_ID);

String DILIGENT_SIGNING_PAYER_ID = "0.0.1340";
AccountID DILIGENT_SIGNING_PAYER = asAccount(DILIGENT_SIGNING_PAYER_ID);
KeyTree DILIGENT_SIGNING_PAYER_KT = withRoot(threshold(2, ed25519(true), ed25519(true), ed25519(false)));
Expand All @@ -349,6 +352,7 @@ default ScheduleStore scheduleStore() {
KeyTree FROM_OVERLAP_PAYER_KT = withRoot(threshold(2, ed25519(true), ed25519(true), ed25519(false)));

KeyTree NEW_ACCOUNT_KT = withRoot(list(ed25519(), threshold(1, ed25519(), ed25519())));
KeyTree SYS_ACCOUNT_KT = withRoot(list(ed25519(), threshold(1, ed25519(), ed25519())));
KeyTree LONG_THRESHOLD_KT = withRoot(threshold(1, ed25519(), ed25519(), ed25519(), ed25519()));

String MISSING_FILE_ID = "1.2.3";
Expand Down
41 changes: 40 additions & 1 deletion test-clients/src/main/java/com/hedera/services/bdd/suites/crypto/MiscCryptoSuite.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.Arrays;
import java.util.List;

import static com.hedera.services.bdd.spec.keys.KeyShape.SIMPLE;
import static com.hederahashgraph.api.proto.java.HederaFunctionality.CryptoTransfer;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INSUFFICIENT_TX_FEE;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INVALID_SIGNATURE;
Expand All @@ -38,6 +39,7 @@
import static com.hedera.services.bdd.spec.utilops.UtilVerbs.*;
import static com.hedera.services.bdd.spec.queries.QueryVerbs.*;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.OK;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.SUCCESS;

public class MiscCryptoSuite extends HapiApiSuite {
anighanta marked this conversation as resolved.
Show resolved Hide resolved
private static final Logger log = LogManager.getLogger(MiscCryptoSuite.class);
Expand All @@ -58,7 +60,8 @@ private List<HapiApiSpec> positiveTests() {
return Arrays.asList(
// transferChangesBalance()
// getsGenesisBalance()
reduceTransferFee()
// reduceTransferFee(),
sysAccountKeyUpdateBySpecialWontNeedNewKeyTxnSign()
);
}
private List<HapiApiSpec> negativeTests() {
Expand All @@ -67,6 +70,42 @@ private List<HapiApiSpec> negativeTests() {
);
}

private HapiApiSpec sysAccountKeyUpdateBySpecialWontNeedNewKeyTxnSign(){
String sysAccount = "0.0.977";
String randomAccountA = "randomAccountA";
String randomAccountB = "randomAccountB";
String firstKey = "firstKey";
String secondKey = "secondKey";

return defaultHapiSpec("sysAccountKeyUpdateBySpecialWontNeedNewKeyTxnSign")
.given(
withOpContext((spec, opLog) -> {
anighanta marked this conversation as resolved.
Show resolved Hide resolved
spec.registry().saveKey(sysAccount, spec.registry().getKey(GENESIS));
}),
newKeyNamed(firstKey).shape(SIMPLE),
newKeyNamed(secondKey).shape(SIMPLE)
)
.when(
cryptoCreate(randomAccountA)
.key(firstKey),
cryptoCreate(randomAccountB)
.key(firstKey)
.balance(ONE_HUNDRED_HBARS)
anighanta marked this conversation as resolved.
Show resolved Hide resolved
)
.then(
cryptoUpdate(sysAccount)
.key(secondKey)
anighanta marked this conversation as resolved.
Show resolved Hide resolved
.payingWith(GENESIS)
.hasKnownStatus(SUCCESS)
.logged(),
cryptoUpdate(randomAccountA)
.key(secondKey)
.signedBy(firstKey)
.payingWith(randomAccountB)
anighanta marked this conversation as resolved.
Show resolved Hide resolved
.hasKnownStatus(INVALID_SIGNATURE)
);
}

private HapiApiSpec reduceTransferFee() {
final long REDUCED_NODE_FEE = 2L;
final long REDUCED_NETWORK_FEE = 3L;
Expand Down