Skip to content
This repository has been archived by the owner on Apr 11, 2021. It is now read-only.

Unit add to Variable Update run-batch-submitter.ts #50

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

walkingbackward
Copy link

Added: MS after POLL_INTERVAL
Added: S after MAX_SUBMISSION_TIME

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Added: MS after POLL_INTERVAL
Added: S after MAX_SUBMISSION_TIME
@@ -167,7 +167,7 @@ export const run = async () => {
parseInt(requiredEnvVars.MIN_TX_SIZE, 10),
parseInt(requiredEnvVars.MAX_TX_SIZE, 10),
parseInt(requiredEnvVars.MAX_BATCH_SIZE, 10),
parseInt(requiredEnvVars.MAX_BATCH_SUBMISSION_TIME, 10) * 1_000,
parseInt(requiredEnvVars.MAX_BATCH_SUBMISSION_TIME_S, 10) * 1_000,
parseInt(requiredEnvVars.NUM_CONFIRMATIONS, 10),
parseInt(requiredEnvVars.RESUBMISSION_TIMEOUT, 10) * 1_000,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also be changed to RESUBMISSION_TIMEOUT_S

@smartcontracts
Copy link

I think we need to change our types somewhere.

@@ -38,9 +38,9 @@ interface RequiredEnvVars {
// The maximum number of L2 transactions that can ever be in a batch.
MAX_BATCH_SIZE: 'MAX_BATCH_SIZE'
// The maximum amount of time (seconds) that we will wait before submitting an under-sized batch.
MAX_BATCH_SUBMISSION_TIME: 'MAX_BATCH_SUBMISSION_TIME'
MAX_BATCH_SUBMISSION_TIME: 'MAX_BATCH_SUBMISSION_TIME_S'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
MAX_BATCH_SUBMISSION_TIME: 'MAX_BATCH_SUBMISSION_TIME_S'
MAX_BATCH_SUBMISSION_TIME_S: 'MAX_BATCH_SUBMISSION_TIME_S'

Ahhh ok I see the issue -- we have to change the keys here too.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same goes for all of the below

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a new pull request with new changes. Tried to edit this existing pull request, but could not figure it out.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants