Skip to content

Commit

Permalink
requested changes
Browse files Browse the repository at this point in the history
  • Loading branch information
Geometer1729 committed Jan 15, 2025
1 parent 537e53f commit ff4b42e
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 50 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
## [Unreleased](https://github.com/o1-labs/o1js/compare/b857516...HEAD)

### Added
- `setFee` and `setFeePerWU` for `Transaction` and `PendingTransaction`
- `setFee` and `setFeePerSnarkCost` for `Transaction` and `PendingTransaction` https://github.com/o1-labs/o1js/pull/1968

### Changed
- Sort order for actions now includes the transaction sequence number and the exact account id sequence https://github.com/o1-labs/o1js/pull/1917
Expand Down
2 changes: 1 addition & 1 deletion src/lib/mina/local-blockchain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ async function LocalBlockchain({
errors,
transaction: txn.transaction,
setFee: txn.setFee,
setFeePerWU: txn.setFeePerWU,
setFeePerSnarkCost: txn.setFeePerSnarkCost,
hash,
toJSON: txn.toJSON,
toPretty: txn.toPretty,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/mina/mina.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ function Network(
errors: updatedErrors,
transaction: txn.transaction,
setFee : txn.setFee,
setFeePerWU : txn.setFeePerWU,
setFeePerSnarkCost : txn.setFeePerSnarkCost,
hash,
toJSON: txn.toJSON,
toPretty: txn.toPretty,
Expand Down
69 changes: 39 additions & 30 deletions src/lib/mina/transaction-validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export {
reportGetAccountError,
defaultNetworkState,
verifyTransactionLimits,
getTotalTimeRequired,
verifyAccountUpdate,
filterGroups,
};
Expand Down Expand Up @@ -58,6 +59,41 @@ function defaultNetworkState(): NetworkValue {
}

function verifyTransactionLimits({ accountUpdates }: ZkappCommand) {

let {totalTimeRequired,eventElements,authTypes} = getTotalTimeRequired(accountUpdates);

let isWithinCostLimit = totalTimeRequired < TransactionCost.COST_LIMIT;

let isWithinEventsLimit =
eventElements.events <= TransactionLimits.MAX_EVENT_ELEMENTS;
let isWithinActionsLimit =
eventElements.actions <= TransactionLimits.MAX_ACTION_ELEMENTS;

let error = '';

if (!isWithinCostLimit) {
// TODO: we should add a link to the docs explaining the reasoning behind it once we have such an explainer
error += `Error: The transaction is too expensive, try reducing the number of AccountUpdates that are attached to the transaction.
Each transaction needs to be processed by the snark workers on the network.
Certain layouts of AccountUpdates require more proving time than others, and therefore are too expensive.
${JSON.stringify(authTypes)}
\n\n`;
}

if (!isWithinEventsLimit) {
error += `Error: The account updates in your transaction are trying to emit too much event data. The maximum allowed number of field elements in events is ${TransactionLimits.MAX_EVENT_ELEMENTS}, but you tried to emit ${eventElements.events}.\n\n`;
}

if (!isWithinActionsLimit) {
error += `Error: The account updates in your transaction are trying to emit too much action data. The maximum allowed number of field elements in actions is ${TransactionLimits.MAX_ACTION_ELEMENTS}, but you tried to emit ${eventElements.actions}.\n\n`;
}

if (error) throw Error('Error during transaction sending:\n\n' + error);
}


function getTotalTimeRequired(accountUpdates : AccountUpdate[]){
let eventElements = { events: 0, actions: 0 };

let authKinds = accountUpdates.map((update) => {
Expand All @@ -83,7 +119,7 @@ function verifyTransactionLimits({ accountUpdates }: ZkappCommand) {
np := proof
n2 := signedPair
n1 := signedSingle
formula used to calculate how expensive a zkapp transaction is
10.26*np + 10.08*n2 + 9.14*n1 < 69.45
Expand All @@ -92,35 +128,8 @@ function verifyTransactionLimits({ accountUpdates }: ZkappCommand) {
TransactionCost.PROOF_COST * authTypes.proof +
TransactionCost.SIGNED_PAIR_COST * authTypes.signedPair +
TransactionCost.SIGNED_SINGLE_COST * authTypes.signedSingle;

let isWithinCostLimit = totalTimeRequired < TransactionCost.COST_LIMIT;

let isWithinEventsLimit =
eventElements.events <= TransactionLimits.MAX_EVENT_ELEMENTS;
let isWithinActionsLimit =
eventElements.actions <= TransactionLimits.MAX_ACTION_ELEMENTS;

let error = '';

if (!isWithinCostLimit) {
// TODO: we should add a link to the docs explaining the reasoning behind it once we have such an explainer
error += `Error: The transaction is too expensive, try reducing the number of AccountUpdates that are attached to the transaction.
Each transaction needs to be processed by the snark workers on the network.
Certain layouts of AccountUpdates require more proving time than others, and therefore are too expensive.
${JSON.stringify(authTypes)}
\n\n`;
}

if (!isWithinEventsLimit) {
error += `Error: The account updates in your transaction are trying to emit too much event data. The maximum allowed number of field elements in events is ${TransactionLimits.MAX_EVENT_ELEMENTS}, but you tried to emit ${eventElements.events}.\n\n`;
}

if (!isWithinActionsLimit) {
error += `Error: The account updates in your transaction are trying to emit too much action data. The maximum allowed number of field elements in actions is ${TransactionLimits.MAX_ACTION_ELEMENTS}, but you tried to emit ${eventElements.actions}.\n\n`;
}

if (error) throw Error('Error during transaction sending:\n\n' + error);
// returns totalTimeRequired and additional data used by verifyTransactionLimits
return {totalTimeRequired,eventElements,authTypes};
}

function countEventElements({ data }: Events) {
Expand Down
8 changes: 4 additions & 4 deletions src/lib/mina/transaction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ describe('transactions', () => {
});
let nonce = tx.transaction.feePayer.body.nonce;
let promise = await tx.sign([feePayer.key, contractAccount.key]).send();
let new_fee = promise.setFee(new UInt64(100));
let new_fee = await promise.setFee(new UInt64(100));
new_fee.sign([feePayer.key,contractAccount.key]);
// second send is rejected for using the same nonce
await expect((new_fee.send()))
.rejects
.toThrowError("Account_nonce_precondition_unsatisfied");
// check that tx was applied, by checking nonce was incremented
expect((await new_fee).transaction.feePayer.body.nonce).toEqual(nonce);
expect(new_fee.transaction.feePayer.body.nonce).toEqual(nonce);
});

it('Second tx should work when first not sent', async () => {
Expand All @@ -57,8 +57,8 @@ describe('transactions', () => {
});
let nonce = tx.transaction.feePayer.body.nonce;
let promise = tx.sign([feePayer.key, contractAccount.key]);
let new_fee = promise.setFee(new UInt64(100));
await new_fee.sign([feePayer.key,contractAccount.key]).prove();
let new_fee = promise.setFeePerSnarkCost(42.7);
await new_fee.sign([feePayer.key,contractAccount.key]);
await new_fee.send();
// check that tx was applied, by checking nonce was incremented
expect((await new_fee).transaction.feePayer.body.nonce).toEqual(nonce);
Expand Down
27 changes: 14 additions & 13 deletions src/lib/mina/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { type SendZkAppResponse, sendZkappQuery } from './graphql.js';
import { type FetchMode } from './transaction-context.js';
import { assertPromise } from '../util/assert.js';
import { Types } from '../../bindings/mina-transaction/types.js';
import { getTotalTimeRequired } from './transaction-validation.js';

export {
Transaction,
Expand Down Expand Up @@ -124,15 +125,15 @@ type Transaction<
* // Waits for some time and decide to resend with a higher fee
*
* tx.setFee(newFee);
* await tx.sign([privateKey]).prove();
* await tx.sign([feePayerKey]));
* await tx.send();
* ```
*/
setFee(newFee:UInt64) : TransactionPromise<false,false>;
setFee(newFee:UInt64) : TransactionPromise<Proven,false>;
/**
* setFeePerWU behaves identically to {@link setFee} but the fee is given per Account Update in the transaction. This is useful because nodes prioritize transactions by fee per weight unit.
* setFeePerSnarkCost behaves identically to {@link setFee} but the fee is given per Account Update in the transaction. This is useful because nodes prioritize transactions by fee per weight unit.
*/
setFeePerWU(newFeePerWU:UInt64) : TransactionPromise<false,false>;
setFeePerSnarkCost(newFeePerSnarkCost:number) : TransactionPromise<Proven,false>;
} & (Proven extends false
? {
/**
Expand Down Expand Up @@ -273,11 +274,11 @@ type PendingTransaction = Pick<
/**
* setFee is the same as {@link Transaction.setFee(newFee)} but for a {@link PendingTransaction}.
*/
setFee(newFee:UInt64):TransactionPromise<false,false>;
setFee(newFee:UInt64):TransactionPromise<boolean,false>;
/**
* setFeePerWU is the same as {@link Transaction.setFeeWU(newFeePerWU)} but for a {@link PendingTransaction}.
* setFeePerSnarkCost is the same as {@link Transaction.setFeeWU(newFeePerWU)} but for a {@link PendingTransaction}.
*/
setFeePerWU(newFeePerWU:UInt64):TransactionPromise<false,false>;
setFeePerSnarkCost(newFeePerSnarkCost:number):TransactionPromise<boolean,false>;
};

/**
Expand Down Expand Up @@ -572,20 +573,20 @@ function newTransaction(transaction: ZkappCommand, proofsEnabled?: boolean) {
}
return pendingTransaction;
},
setFeePerWU(newFeePerWU:UInt64) {
//Currently WU is just the number of accountUpdates + 1
//https://github.com/MinaProtocol/mina/blob/a0a2adf6b1dce7af889250ff469a35ae4afa512f/src/lib/mina_base/zkapp_command.ml#L803-L823
//The code reads like a placeholder, so ideally we should update this if it changes
return this.setFee(newFeePerWU.mul(new UInt64(this.transaction.accountUpdates.length + 1)))
setFeePerSnarkCost(newFeePerSnarkCost:number) {
let {totalTimeRequired} = getTotalTimeRequired(transaction.accountUpdates);
return this.setFee(new UInt64(Math.round(totalTimeRequired * newFeePerSnarkCost)));
},
setFee(newFee:UInt64) {
return toTransactionPromise(async () =>
{
self = self as Transaction<false,false>;
self.transaction.accountUpdates.forEach( au => {
au.authorization.proof = undefined;
if (au.body.useFullCommitment)
{
au.authorization.signature = undefined;
au.lazyAuthorization = {kind:'lazy-signature'};
}
});
self.transaction.feePayer.body.fee = newFee;
self.transaction.feePayer.lazyAuthorization = {kind : 'lazy-signature'};
Expand Down

0 comments on commit ff4b42e

Please sign in to comment.