Skip to content

Commit ff4b42e

Browse files
committed
requested changes
1 parent 537e53f commit ff4b42e

File tree

6 files changed

+60
-50
lines changed

6 files changed

+60
-50
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
1818
## [Unreleased](https://github.com/o1-labs/o1js/compare/b857516...HEAD)
1919

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

2323
### Changed
2424
- Sort order for actions now includes the transaction sequence number and the exact account id sequence https://github.com/o1-labs/o1js/pull/1917

src/lib/mina/local-blockchain.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ async function LocalBlockchain({
294294
errors,
295295
transaction: txn.transaction,
296296
setFee: txn.setFee,
297-
setFeePerWU: txn.setFeePerWU,
297+
setFeePerSnarkCost: txn.setFeePerSnarkCost,
298298
hash,
299299
toJSON: txn.toJSON,
300300
toPretty: txn.toPretty,

src/lib/mina/mina.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ function Network(
283283
errors: updatedErrors,
284284
transaction: txn.transaction,
285285
setFee : txn.setFee,
286-
setFeePerWU : txn.setFeePerWU,
286+
setFeePerSnarkCost : txn.setFeePerSnarkCost,
287287
hash,
288288
toJSON: txn.toJSON,
289289
toPretty: txn.toPretty,

src/lib/mina/transaction-validation.ts

Lines changed: 39 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ export {
2626
reportGetAccountError,
2727
defaultNetworkState,
2828
verifyTransactionLimits,
29+
getTotalTimeRequired,
2930
verifyAccountUpdate,
3031
filterGroups,
3132
};
@@ -58,6 +59,41 @@ function defaultNetworkState(): NetworkValue {
5859
}
5960

6061
function verifyTransactionLimits({ accountUpdates }: ZkappCommand) {
62+
63+
let {totalTimeRequired,eventElements,authTypes} = getTotalTimeRequired(accountUpdates);
64+
65+
let isWithinCostLimit = totalTimeRequired < TransactionCost.COST_LIMIT;
66+
67+
let isWithinEventsLimit =
68+
eventElements.events <= TransactionLimits.MAX_EVENT_ELEMENTS;
69+
let isWithinActionsLimit =
70+
eventElements.actions <= TransactionLimits.MAX_ACTION_ELEMENTS;
71+
72+
let error = '';
73+
74+
if (!isWithinCostLimit) {
75+
// TODO: we should add a link to the docs explaining the reasoning behind it once we have such an explainer
76+
error += `Error: The transaction is too expensive, try reducing the number of AccountUpdates that are attached to the transaction.
77+
Each transaction needs to be processed by the snark workers on the network.
78+
Certain layouts of AccountUpdates require more proving time than others, and therefore are too expensive.
79+
80+
${JSON.stringify(authTypes)}
81+
\n\n`;
82+
}
83+
84+
if (!isWithinEventsLimit) {
85+
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`;
86+
}
87+
88+
if (!isWithinActionsLimit) {
89+
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`;
90+
}
91+
92+
if (error) throw Error('Error during transaction sending:\n\n' + error);
93+
}
94+
95+
96+
function getTotalTimeRequired(accountUpdates : AccountUpdate[]){
6197
let eventElements = { events: 0, actions: 0 };
6298

6399
let authKinds = accountUpdates.map((update) => {
@@ -83,7 +119,7 @@ function verifyTransactionLimits({ accountUpdates }: ZkappCommand) {
83119
np := proof
84120
n2 := signedPair
85121
n1 := signedSingle
86-
122+
87123
formula used to calculate how expensive a zkapp transaction is
88124
89125
10.26*np + 10.08*n2 + 9.14*n1 < 69.45
@@ -92,35 +128,8 @@ function verifyTransactionLimits({ accountUpdates }: ZkappCommand) {
92128
TransactionCost.PROOF_COST * authTypes.proof +
93129
TransactionCost.SIGNED_PAIR_COST * authTypes.signedPair +
94130
TransactionCost.SIGNED_SINGLE_COST * authTypes.signedSingle;
95-
96-
let isWithinCostLimit = totalTimeRequired < TransactionCost.COST_LIMIT;
97-
98-
let isWithinEventsLimit =
99-
eventElements.events <= TransactionLimits.MAX_EVENT_ELEMENTS;
100-
let isWithinActionsLimit =
101-
eventElements.actions <= TransactionLimits.MAX_ACTION_ELEMENTS;
102-
103-
let error = '';
104-
105-
if (!isWithinCostLimit) {
106-
// TODO: we should add a link to the docs explaining the reasoning behind it once we have such an explainer
107-
error += `Error: The transaction is too expensive, try reducing the number of AccountUpdates that are attached to the transaction.
108-
Each transaction needs to be processed by the snark workers on the network.
109-
Certain layouts of AccountUpdates require more proving time than others, and therefore are too expensive.
110-
111-
${JSON.stringify(authTypes)}
112-
\n\n`;
113-
}
114-
115-
if (!isWithinEventsLimit) {
116-
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`;
117-
}
118-
119-
if (!isWithinActionsLimit) {
120-
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`;
121-
}
122-
123-
if (error) throw Error('Error during transaction sending:\n\n' + error);
131+
// returns totalTimeRequired and additional data used by verifyTransactionLimits
132+
return {totalTimeRequired,eventElements,authTypes};
124133
}
125134

126135
function countEventElements({ data }: Events) {

src/lib/mina/transaction.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@ describe('transactions', () => {
4040
});
4141
let nonce = tx.transaction.feePayer.body.nonce;
4242
let promise = await tx.sign([feePayer.key, contractAccount.key]).send();
43-
let new_fee = promise.setFee(new UInt64(100));
43+
let new_fee = await promise.setFee(new UInt64(100));
4444
new_fee.sign([feePayer.key,contractAccount.key]);
4545
// second send is rejected for using the same nonce
4646
await expect((new_fee.send()))
4747
.rejects
4848
.toThrowError("Account_nonce_precondition_unsatisfied");
4949
// check that tx was applied, by checking nonce was incremented
50-
expect((await new_fee).transaction.feePayer.body.nonce).toEqual(nonce);
50+
expect(new_fee.transaction.feePayer.body.nonce).toEqual(nonce);
5151
});
5252

5353
it('Second tx should work when first not sent', async () => {
@@ -57,8 +57,8 @@ describe('transactions', () => {
5757
});
5858
let nonce = tx.transaction.feePayer.body.nonce;
5959
let promise = tx.sign([feePayer.key, contractAccount.key]);
60-
let new_fee = promise.setFee(new UInt64(100));
61-
await new_fee.sign([feePayer.key,contractAccount.key]).prove();
60+
let new_fee = promise.setFeePerSnarkCost(42.7);
61+
await new_fee.sign([feePayer.key,contractAccount.key]);
6262
await new_fee.send();
6363
// check that tx was applied, by checking nonce was incremented
6464
expect((await new_fee).transaction.feePayer.body.nonce).toEqual(nonce);

src/lib/mina/transaction.ts

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { type SendZkAppResponse, sendZkappQuery } from './graphql.js';
2323
import { type FetchMode } from './transaction-context.js';
2424
import { assertPromise } from '../util/assert.js';
2525
import { Types } from '../../bindings/mina-transaction/types.js';
26+
import { getTotalTimeRequired } from './transaction-validation.js';
2627

2728
export {
2829
Transaction,
@@ -124,15 +125,15 @@ type Transaction<
124125
* // Waits for some time and decide to resend with a higher fee
125126
*
126127
* tx.setFee(newFee);
127-
* await tx.sign([privateKey]).prove();
128+
* await tx.sign([feePayerKey]));
128129
* await tx.send();
129130
* ```
130131
*/
131-
setFee(newFee:UInt64) : TransactionPromise<false,false>;
132+
setFee(newFee:UInt64) : TransactionPromise<Proven,false>;
132133
/**
133-
* 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.
134+
* 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.
134135
*/
135-
setFeePerWU(newFeePerWU:UInt64) : TransactionPromise<false,false>;
136+
setFeePerSnarkCost(newFeePerSnarkCost:number) : TransactionPromise<Proven,false>;
136137
} & (Proven extends false
137138
? {
138139
/**
@@ -273,11 +274,11 @@ type PendingTransaction = Pick<
273274
/**
274275
* setFee is the same as {@link Transaction.setFee(newFee)} but for a {@link PendingTransaction}.
275276
*/
276-
setFee(newFee:UInt64):TransactionPromise<false,false>;
277+
setFee(newFee:UInt64):TransactionPromise<boolean,false>;
277278
/**
278-
* setFeePerWU is the same as {@link Transaction.setFeeWU(newFeePerWU)} but for a {@link PendingTransaction}.
279+
* setFeePerSnarkCost is the same as {@link Transaction.setFeeWU(newFeePerWU)} but for a {@link PendingTransaction}.
279280
*/
280-
setFeePerWU(newFeePerWU:UInt64):TransactionPromise<false,false>;
281+
setFeePerSnarkCost(newFeePerSnarkCost:number):TransactionPromise<boolean,false>;
281282
};
282283

283284
/**
@@ -572,20 +573,20 @@ function newTransaction(transaction: ZkappCommand, proofsEnabled?: boolean) {
572573
}
573574
return pendingTransaction;
574575
},
575-
setFeePerWU(newFeePerWU:UInt64) {
576-
//Currently WU is just the number of accountUpdates + 1
577-
//https://github.com/MinaProtocol/mina/blob/a0a2adf6b1dce7af889250ff469a35ae4afa512f/src/lib/mina_base/zkapp_command.ml#L803-L823
578-
//The code reads like a placeholder, so ideally we should update this if it changes
579-
return this.setFee(newFeePerWU.mul(new UInt64(this.transaction.accountUpdates.length + 1)))
576+
setFeePerSnarkCost(newFeePerSnarkCost:number) {
577+
let {totalTimeRequired} = getTotalTimeRequired(transaction.accountUpdates);
578+
return this.setFee(new UInt64(Math.round(totalTimeRequired * newFeePerSnarkCost)));
580579
},
581580
setFee(newFee:UInt64) {
582581
return toTransactionPromise(async () =>
583582
{
584583
self = self as Transaction<false,false>;
585584
self.transaction.accountUpdates.forEach( au => {
586-
au.authorization.proof = undefined;
585+
if (au.body.useFullCommitment)
586+
{
587587
au.authorization.signature = undefined;
588588
au.lazyAuthorization = {kind:'lazy-signature'};
589+
}
589590
});
590591
self.transaction.feePayer.body.fee = newFee;
591592
self.transaction.feePayer.lazyAuthorization = {kind : 'lazy-signature'};

0 commit comments

Comments
 (0)