feat: use zod for sequencer config #5419
Conversation
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
alexghr
force-pushed
the
03-24-feat_use_zod_for_config
branch
2 times, most recently
from
195294c to
abce5b6
Compare
Benchmark resultsNo metrics with a significant change found. Detailed resultsAll benchmarks are run on txs on the This benchmark source data is available in JSON format on S3 here. Values are compared against data from master at commit L2 block published to L1Each column represents the number of txs on an L2 block published to L1.
L2 chain processingEach column represents the number of blocks on the L2 chain where each block has 16 txs.
Circuits statsStats on running time and I/O sizes collected for every circuit run across all benchmarks.
Tree insertion statsThe duration to insert a fixed batch of leaves into each tree type.
MiscellaneousTransaction sizes based on how many contract classes are registered in the tx.
Transaction size based on fee payment method
Transaction processing duration by data writes.
|
alexghr
force-pushed
the
03-24-feat_use_zod_for_config
branch
from
abce5b6 to
a60c28f
Compare
| return true; | ||
| } | ||
|
|
||
| if ('buffer' in value && Buffer.isBuffer(value.buffer) && value.buffer.length === EthAddress.SIZE_IN_BYTES) { |
There was a problem hiding this comment.
nit: might be safer here to try/catch fromBuffer. this is making assumptions about the validity checks in the constructor.
There was a problem hiding this comment.
If I understand correctly, would it look something like:
try {
if ('toBuffer' in value && EthAddress.fromBuffer(value.toBuffer()) {
return true;
}
} catch {
return false;
}I think that will work and be safe but we'd be creating objects and discarding them immediately.
There was a problem hiding this comment.
That is what I had meant, but perhaps the best approach is to extract the validation code to a function that both this typeguard and the constructor use?
|
|
||
| /** | ||
| * The RPC Url of the ethereum host. | ||
| */ | ||
| rpcUrl: string; | ||
| rpcUrl: z.string().optional().default(''), |
There was a problem hiding this comment.
Not sure if you were aware, zod does have a url validator. Saw this comment on github
z.string().url().optional().or(z.literal(''))
There was a problem hiding this comment.
Oh that's great. I hadn't considered enforcing it be an url
| l1BlockPublishRetryIntervalMS: SEQ_PUBLISH_RETRY_INTERVAL_MS ? +SEQ_PUBLISH_RETRY_INTERVAL_MS : 1_000, | ||
| transactionPollingIntervalMS: SEQ_TX_POLLING_INTERVAL_MS ? +SEQ_TX_POLLING_INTERVAL_MS : 1_000, | ||
| l1Contracts: addresses, | ||
| return sequencerClientConfig.parse({ |
yarn-project/aztec/src/cli/util.ts
Outdated
| const [key, value] = option.split('='); | ||
| const [key, value] = options.includes('=') | ||
| ? option.split('=') | ||
| : option.includes(':') |
There was a problem hiding this comment.
Why do we need the colon variety?
There was a problem hiding this comment.
Honestly we don't but while testing I kept doing using colon instead of equals. i'll remove
alexghr
force-pushed
the
03-24-feat_use_zod_for_config
branch
from
a60c28f to
e51a21d
Compare
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is an install script?Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts. Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
alexghr
force-pushed
the
03-24-feat_use_zod_for_config
branch
2 times, most recently
from
034b9b8 to
36ae57f
Compare
|
I'm not sure what's going on with Socket but there are no changes to any package.json files or to yarn.lock |
alexghr
force-pushed
the
03-24-feat_use_zod_for_config
branch
from
36ae57f to
e27cd65
Compare
|
I'll close this PR and we can revisit at a later time |
This PR uses zod for validating the sequencer's configuration. See #5310 (comment) for more info