Add setFee and setFeePerWU #1968
Conversation
|
This PR clears the proof and signature authorizations for all AccountUpdates. At the same time, for many txs, only the signature of the feePayer should be changed. Is it possible to differentiate between the signatures and proofs that should be changed in case of fee change and those that can stay the same? |
I would go even further: It's currently not possibly that any proofs are invalidated by a fee change, because they can't refer to the fee payer. Signatures are affected if and only if they have So you can actually be fully precise with invalidation. |
CHANGELOG.md
Outdated
| @@ -17,6 +17,9 @@ 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` | |||
There was a problem hiding this comment.
Can you add the link to this PR to this changelog entry?
src/lib/mina/transaction.ts
Outdated
| /** | ||
| * setFeePerWU is the same as {@link Transaction.setFeeWU(newFeePerWU)} but for a {@link PendingTransaction}. | ||
| */ | ||
| setFeePerWU(newFeePerWU:UInt64):TransactionPromise<false,false>; |
There was a problem hiding this comment.
[nit] As a style choice, I think setFeePerWeightUnit is a better choice for the name of this method.
src/lib/mina/transaction.ts
Outdated
| @@ -544,6 +572,26 @@ 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 | |||
There was a problem hiding this comment.
This is the weight of a zkapp_command, which ends up being different than the weight of a signed command:
Also, there is this other formula for "cost", which I think is the most relevant formula to use for this context: https://github.com/MinaProtocol/mina/blob/a0a2adf6b1dce7af889250ff469a35ae4afa512f/src/lib/mina_base/zkapp_command.ml#L1317, ( duplicated in o1js here:
)(*10.26*np + 10.08*n2 + 9.14*n1 < 69.45*)
The way I understand it, this formula estimates how long it will take to snark a transaction, which is turn is what makes snark workers want more payment. Number of account updates +1 doesn't count proofs at all. I think we should disambiguate "cost" and "weight" to make sure we are capturing the right thing with this new method.
Co-authored-by: Anaïs Querol <[email protected]>
|
|
src/lib/mina/transaction.ts
Outdated
| */ | ||
| setFee(newFee:UInt64) : TransactionPromise<Proven,false>; | ||
| /** | ||
| * 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. |
There was a problem hiding this comment.
but the fee is given per Account Update in the transaction. This is useful because nodes prioritize transactions by fee per weight unit.
This seems to describe the previous behavior. Should be updated to mention snark cost instead of weight unit, and mention proofs as well as AccountUpdates. Might be nice to {@link getTotalTimeRequired} as well, but I'm not sure that's a public function.
There was a problem hiding this comment.
The link at least works in my LSP
src/lib/mina/transaction.ts
Outdated
| */ | ||
| setFee(newFee:UInt64):TransactionPromise<boolean,false>; | ||
| /** | ||
| * setFeePerSnarkCost is the same as {@link Transaction.setFeeWU(newFeePerWU)} but for a {@link PendingTransaction}. |
|
Thanks everyone, for bearing with me and catching all those mistakes. |
Closes #1626