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.

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

Enable custom network signing #1444

Merged
merged 23 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased](https://github.com/o1-labs/o1js/compare/3b5f7c7...HEAD)

### Added

- Support for custom network identifiers other than `mainnet` or `testnet` https://github.com/o1-labs/o1js/pull/1444

## [0.16.1](https://github.com/o1-labs/o1js/compare/834a44002...3b5f7c7)

### Breaking changes
Expand Down
17 changes: 9 additions & 8 deletions src/lib/account-update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ import {
protocolVersions,
} from '../bindings/crypto/constants.js';
import { MlArray } from './ml/base.js';
import { Signature, signFieldElement } from '../mina-signer/src/signature.js';
import {
Signature,
signFieldElement,
zkAppBodyPrefix,
} from '../mina-signer/src/signature.js';
import { MlFieldConstArray } from './ml/fields.js';
import {
accountUpdatesToCallForest,
Expand All @@ -65,6 +69,7 @@ import {
} from './mina/smart-contract-context.js';
import { assert } from './util/assert.js';
import { RandomId } from './provable-types/auxiliary.js';
import { NetworkId } from '../mina-signer/src/types.js';

// external API
export {
Expand Down Expand Up @@ -1018,17 +1023,15 @@ class AccountUpdate implements Types.AccountUpdate {
if (Provable.inCheckedComputation()) {
let input = Types.AccountUpdate.toInput(this);
return hashWithPrefix(
activeInstance.getNetworkId() === 'mainnet'
? prefixes.zkappBodyMainnet
: prefixes.zkappBodyTestnet,
zkAppBodyPrefix(activeInstance.getNetworkId()),
packToFields(input)
);
} else {
let json = Types.AccountUpdate.toJSON(this);
return Field(
Test.hashFromJson.accountUpdate(
JSON.stringify(json),
activeInstance.getNetworkId()
NetworkId.toString(activeInstance.getNetworkId())
)
);
}
Expand Down Expand Up @@ -1399,9 +1402,7 @@ class AccountUpdate implements Types.AccountUpdate {
function hashAccountUpdate(update: AccountUpdate) {
return genericHash(
AccountUpdate,
activeInstance.getNetworkId() === 'mainnet'
? prefixes.zkappBodyMainnet
: prefixes.zkappBodyTestnet,
zkAppBodyPrefix(activeInstance.getNetworkId()),
update
);
}
Expand Down
11 changes: 9 additions & 2 deletions src/lib/signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { hashWithPrefix } from './hash.js';
import {
deriveNonce,
Signature as SignatureBigint,
signaturePrefix,
} from '../mina-signer/src/signature.js';
import { Bool as BoolBigint } from '../provable/field-bigint.js';
import {
Expand Down Expand Up @@ -238,6 +239,9 @@ class Signature extends CircuitValue {
static create(privKey: PrivateKey, msg: Field[]): Signature {
const publicKey = PublicKey.fromPrivateKey(privKey).toGroup();
const d = privKey.s;
// we chose an arbitrary prefix for the signature, and it happened to be 'testnet'
// there's no consequences in practice and the signatures can be used with any network
// if there needs to be a custom nonce, include it in the message itself
const kPrime = Scalar.fromBigInt(
deriveNonce(
{ fields: msg.map((f) => f.toBigInt()) },
Expand All @@ -249,7 +253,7 @@ class Signature extends CircuitValue {
let { x: r, y: ry } = Group.generator.scale(kPrime);
const k = ry.toBits()[0].toBoolean() ? kPrime.neg() : kPrime;
let h = hashWithPrefix(
prefixes.signatureTestnet,
signaturePrefix('testnet'),
msg.concat([publicKey.x, publicKey.y, r])
);
// TODO: Scalar.fromBits interprets the input as a "shifted scalar"
Expand All @@ -265,8 +269,11 @@ class Signature extends CircuitValue {
*/
verify(publicKey: PublicKey, msg: Field[]): Bool {
const point = publicKey.toGroup();
// we chose an arbitrary prefix for the signature, and it happened to be 'testnet'
// there's no consequences in practice and the signatures can be used with any network
// if there needs to be a custom nonce, include it in the message itself
let h = hashWithPrefix(
prefixes.signatureTestnet,
signaturePrefix('testnet'),
msg.concat([point.x, point.y, this.r])
);
// TODO: Scalar.fromBits interprets the input as a "shifted scalar"
Expand Down
13 changes: 3 additions & 10 deletions src/mina-signer/mina-signer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,10 @@ export { Client, Client as default, type NetworkId };
const defaultValidUntil = '4294967295';

class Client {
private network: NetworkId; // TODO: Rename to "networkId" for consistency with remaining codebase.
private network: NetworkId;

constructor(options: { network: NetworkId }) {
if (!options?.network) {
throw Error('Invalid Specified Network');
}
const specifiedNetwork = options.network.toLowerCase();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is no longer needed since the type now is 'testnet' | 'mainnet' which already is lower case

if (specifiedNetwork !== 'mainnet' && specifiedNetwork !== 'testnet') {
throw Error('Invalid Specified Network');
}
this.network = specifiedNetwork;
constructor({ network }: { network: NetworkId }) {
this.network = network;
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/mina-signer/src/random-transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ const RandomTransaction = {
zkappCommand,
zkappCommandAndFeePayerKey,
zkappCommandJson,
networkId: Random.oneOf<NetworkId[]>('testnet', 'mainnet'),
networkId: Random.oneOf<NetworkId[]>('testnet', 'mainnet', {
custom: 'other',
}),
accountUpdateWithCallDepth: accountUpdate,
};
2 changes: 1 addition & 1 deletion src/mina-signer/src/sign-legacy.unit-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ let networks: NetworkId[] = ['testnet', 'mainnet'];

for (let network of networks) {
let i = 0;
let reference = signatures[network];
let reference = signatures[NetworkId.toString(network)];

for (let payment of payments) {
let signature = signPayment(payment, privateKey, network);
Expand Down
13 changes: 1 addition & 12 deletions src/mina-signer/src/sign-zkapp-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
Signature,
signFieldElement,
verifyFieldElement,
zkAppBodyPrefix,
} from './signature.js';
import { mocks } from '../../bindings/crypto/constants.js';
import { NetworkId } from './types.js';
Expand Down Expand Up @@ -159,18 +160,6 @@ function accountUpdatesToCallForest<A extends { body: { callDepth: number } }>(
return forest;
}

const zkAppBodyPrefix = (network: string) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved this into mina-signer/src/signature.js for shared use with o1js

switch (network) {
case 'mainnet':
return prefixes.zkappBodyMainnet;
case 'testnet':
return prefixes.zkappBodyTestnet;

default:
return 'ZkappBody' + network;
}
};

function accountUpdateHash(update: AccountUpdate, networkId: NetworkId) {
assertAuthorizationKindValid(update);
let input = AccountUpdate.toInput(update);
Expand Down
112 changes: 61 additions & 51 deletions src/mina-signer/src/sign-zkapp-command.unit-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,42 +82,46 @@ expect(stringify(dummyInput.packed)).toEqual(
stringify(dummyInputSnarky.packed)
);

test(Random.accountUpdate, (accountUpdate) => {
const testnetMinaInstance = Network({
networkId: 'testnet',
mina: 'http://localhost:8080/graphql',
});
const mainnetMinaInstance = Network({
networkId: 'mainnet',
mina: 'http://localhost:8080/graphql',
});

fixVerificationKey(accountUpdate);

// example account update
let accountUpdateJson: Json.AccountUpdate =
AccountUpdate.toJSON(accountUpdate);

// account update hash
let accountUpdateSnarky = AccountUpdateSnarky.fromJSON(accountUpdateJson);
let inputSnarky = TypesSnarky.AccountUpdate.toInput(accountUpdateSnarky);
let input = AccountUpdate.toInput(accountUpdate);
expect(toJSON(input.fields)).toEqual(toJSON(inputSnarky.fields));
expect(toJSON(input.packed)).toEqual(toJSON(inputSnarky.packed));

let packed = packToFields(input);
let packedSnarky = packToFieldsSnarky(inputSnarky);
expect(toJSON(packed)).toEqual(toJSON(packedSnarky));

let hashTestnet = accountUpdateHash(accountUpdate, 'testnet');
let hashMainnet = accountUpdateHash(accountUpdate, 'mainnet');
setActiveInstance(testnetMinaInstance);
let hashSnarkyTestnet = accountUpdateSnarky.hash();
setActiveInstance(mainnetMinaInstance);
let hashSnarkyMainnet = accountUpdateSnarky.hash();
expect(hashTestnet).toEqual(hashSnarkyTestnet.toBigInt());
expect(hashMainnet).toEqual(hashSnarkyMainnet.toBigInt());
});
test(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleaned this up a little by utilizing RandomTransaction.networkId

Copy link
Member

@shimkiv shimkiv Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Trivo25
Throughout different tests we check that signatures match like testnet <-> testnet but should we also check that the resulting signatures are indeed different for different networks? Like testnet <-> mainnet should always differ.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should! And we do
For example, further down we verify against a different network and expect the verification to fail

expect(
   verify(sigFieldElements, networkId === 'mainnet' ? 'testnet' : 'mainnet')
).toEqual(false);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but I added a check that the hash doesnt match another networks hash!

Random.accountUpdate,
RandomTransaction.networkId,
(accountUpdate, networkId) => {
const minaInstance = Network({
networkId,
mina: 'http://localhost:8080/graphql',
});

fixVerificationKey(accountUpdate);

// example account update
let accountUpdateJson: Json.AccountUpdate =
AccountUpdate.toJSON(accountUpdate);

// account update hash
let accountUpdateSnarky = AccountUpdateSnarky.fromJSON(accountUpdateJson);
let inputSnarky = TypesSnarky.AccountUpdate.toInput(accountUpdateSnarky);
let input = AccountUpdate.toInput(accountUpdate);
expect(toJSON(input.fields)).toEqual(toJSON(inputSnarky.fields));
expect(toJSON(input.packed)).toEqual(toJSON(inputSnarky.packed));

let packed = packToFields(input);
let packedSnarky = packToFieldsSnarky(inputSnarky);
expect(toJSON(packed)).toEqual(toJSON(packedSnarky));

setActiveInstance(minaInstance);
let hashSnarky = accountUpdateSnarky.hash();
let hash = accountUpdateHash(accountUpdate, networkId);
expect(hash).toEqual(hashSnarky.toBigInt());

// check against different network hash
expect(hash).not.toEqual(
accountUpdateHash(
accountUpdate,
NetworkId.toString(networkId) === 'mainnet' ? 'testnet' : 'mainnet'
)
);
}
);

// private key to/from base58
test(Random.json.privateKey, (feePayerKeyBase58) => {
Expand All @@ -140,19 +144,25 @@ test(memoGenerator, (memoString) => {
});

// zkapp transaction - basic properties & commitment
test(RandomTransaction.zkappCommand, (zkappCommand, assert) => {
zkappCommand.accountUpdates.forEach(fixVerificationKey);

assert(isCallDepthValid(zkappCommand));
let zkappCommandJson = ZkappCommand.toJSON(zkappCommand);
let ocamlCommitments = Test.hashFromJson.transactionCommitments(
JSON.stringify(zkappCommandJson),
'testnet'
);
let callForest = accountUpdatesToCallForest(zkappCommand.accountUpdates);
let commitment = callForestHash(callForest, 'testnet');
expect(commitment).toEqual(FieldConst.toBigint(ocamlCommitments.commitment));
});
test(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this now tests all network id types

RandomTransaction.zkappCommand,
RandomTransaction.networkId,
(zkappCommand, networkId, assert) => {
zkappCommand.accountUpdates.forEach(fixVerificationKey);

assert(isCallDepthValid(zkappCommand));
let zkappCommandJson = ZkappCommand.toJSON(zkappCommand);
let ocamlCommitments = Test.hashFromJson.transactionCommitments(
JSON.stringify(zkappCommandJson),
NetworkId.toString(networkId)
);
let callForest = accountUpdatesToCallForest(zkappCommand.accountUpdates);
let commitment = callForestHash(callForest, networkId);
expect(commitment).toEqual(
FieldConst.toBigint(ocamlCommitments.commitment)
);
}
);

// invalid zkapp transactions
test.negative(
Expand Down Expand Up @@ -192,7 +202,7 @@ test(
// tx commitment
let ocamlCommitments = Test.hashFromJson.transactionCommitments(
JSON.stringify(zkappCommandJson),
networkId
NetworkId.toString(networkId)
);
let callForest = accountUpdatesToCallForest(zkappCommand.accountUpdates);
let commitment = callForestHash(callForest, networkId);
Expand Down Expand Up @@ -242,7 +252,7 @@ test(
let sigOCaml = Test.signature.signFieldElement(
ocamlCommitments.fullCommitment,
Ml.fromPrivateKey(feePayerKeySnarky),
networkId === 'mainnet' ? true : false
NetworkId.toString(networkId)
);

expect(Signature.toBase58(sigFieldElements)).toEqual(sigOCaml);
Expand Down
Loading
Loading