Unlimited account update support #1919
Conversation
There was a problem hiding this comment.
Thanks for the PR!
I will do a more thorough review before approving, but I think this looks good. The one consideration is that it doesn't allow for a dev to set their own custom transaction limits. It's only an on/off. Does zeko not have transaction limits at all?
src/lib/mina/mina.ts
Outdated
| @@ -158,6 +161,11 @@ function Network( | |||
| lightnetAccountManagerEndpoint = options.lightnetAccountManager; | |||
| Fetch.setLightnetAccountManagerEndpoint(lightnetAccountManagerEndpoint); | |||
| } | |||
|
|
|||
| if (options.enforceTransactionLimits !== undefined && | |||
There was a problem hiding this comment.
I think it would be cleaner to name the option bypassTransactionLimits. That way undefined and false both resolve to the default behavior. Then I think this could be
enforceTransactionLimits = options.bypassTransactionLimits ? false : true;
There was a problem hiding this comment.
I tried to reuse the same name than LocalBlockchain :
async function LocalBlockchain({ proofsEnabled = true, enforceTransactionLimits = true, } = {}) {
o1js/src/lib/mina/local-blockchain.ts
Line 74 in a0f0f34
src/lib/mina/mina.ts
Outdated
| if (options.enforceTransactionLimits !== undefined && | ||
| typeof options.enforceTransactionLimits === 'boolean') { | ||
| enforceTransactionLimits = options.enforceTransactionLimits; | ||
| } |
There was a problem hiding this comment.
| if (options.enforceTransactionLimits !== undefined && | |
| typeof options.enforceTransactionLimits === 'boolean') { | |
| enforceTransactionLimits = options.enforceTransactionLimits; | |
| } | |
| const byPassTransactionLimits = options.enforceTransactionLimits === false; | |
| enforceTransactionLimits = !byPassTransactionLimits; |
I still think this would be more understandable, while respecting the argument agreement with the other function
|
Hey @youtpout, can you please merge the |
Done |
|
@shimkiv, do you know what's wrong with the benchmark o1js job? It looks like it build o1js then immediately fails. |
|
@45930 it is related to self-hosted runners, repo CI config and external contributors. Back then we decided that it is not a big issue and didn't provide a fix (if it exists). |
|
@45930 for example, this particular job relies on repo secrets that external contributors simply don't have, hence the job is failing. |
Some blockchain like zeko are not limited by account update, to enable transaction creation for these blockchains, I've added a new parameter to the network property
enforceTransactionLimitsas for local blockchain.Resolve #1910