Skip to content

Conversation

@MarcosNicolau
Copy link
Member

Description

Moves the createNewTask simulation to run before uploading the batch to S3.

Testing

Run a local devnet and make it revert by:

  • Sending a repeated batchMerkleRoot
  • Updating the BatcherPaymentService contract to trigger a revert

You should see that the batch isn't uploaded to s3 when the simulation fails.

Type of change

  • Refactor

Checklist

Copy link
Contributor

@uri-99 uri-99 left a comment

Choose a reason for hiding this comment

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

code looks good, need to test

@PatStiles PatStiles changed the base branch from staging to testnet December 5, 2024 13:41
@PatStiles PatStiles changed the base branch from testnet to staging December 5, 2024 13:42
Copy link
Contributor

@avilagaston9 avilagaston9 left a comment

Choose a reason for hiding this comment

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

Works in my machine!

Copy link
Contributor

@JulianVentura JulianVentura left a comment

Choose a reason for hiding this comment

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

Code looks fine and worked as expected locally

@uri-99 uri-99 added this pull request to the merge queue Dec 6, 2024
github-merge-queue bot pushed a commit that referenced this pull request Dec 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 6, 2024
@uri-99 uri-99 added this pull request to the merge queue Dec 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Dec 6, 2024
@PatStiles PatStiles added this pull request to the merge queue Dec 6, 2024
@PatStiles PatStiles removed this pull request from the merge queue due to a manual request Dec 6, 2024
@JuArce JuArce added this pull request to the merge queue Dec 10, 2024
@JuArce JuArce removed this pull request from the merge queue due to a manual request Dec 10, 2024
@JuArce JuArce added this pull request to the merge queue Dec 10, 2024
Merged via the queue into staging with commit 75c34a8 Dec 10, 2024
3 checks passed
@JuArce JuArce deleted the 1524-fixbatcher-createtask-simulation-should-be-executed-before-sending-batch-to-s3 branch December 10, 2024 14:40
PatStiles pushed a commit that referenced this pull request Jan 10, 2025
PatStiles pushed a commit that referenced this pull request Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(batcher): createTask simulation should be executed before sending batch to s3

7 participants