Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sdk): Add missing message status index #6254

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

roninjin10
Copy link
Contributor

  • missed a few message index updates in a previous pr

@roninjin10 roninjin10 requested a review from a team as a code owner July 11, 2023 14:55
@changeset-bot
Copy link

changeset-bot bot commented Jul 11, 2023

🦋 Changeset detected

Latest commit: a666c4f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@eth-optimism/sdk Patch
@eth-optimism/chain-mon Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@roninjin10
Copy link
Contributor Author

roninjin10 commented Jul 11, 2023

@mergify mergify bot added the sdk label Jul 11, 2023
@semgrep-app
Copy link
Contributor

semgrep-app bot commented Jul 11, 2023

Semgrep found 1 nondeterministic-select finding:

  • op-proposer/proposer/l2_output_submitter.go: L350-375

Logic executed as a result of ticker ticker may execute more times than desired.
When both ticker and l.done are written to at the same time, the scheduler randomly picks a
case to execute. As a result, the ticker.C may excute one more time than expected.

Ignore this finding from nondeterministic-select.

@netlify
Copy link

netlify bot commented Jul 11, 2023

Deploy Preview for opstack-docs canceled.

Name Link
🔨 Latest commit a666c4f
🔍 Latest deploy log https://app.netlify.com/sites/opstack-docs/deploys/64ad97c018d77c000813f63c

@roninjin10 roninjin10 force-pushed the 07-11-fix_sdk_Add_missing_message_statuses branch from 30c20ea to 583ee4f Compare July 11, 2023 15:03
@roninjin10 roninjin10 changed the title fix(sdk): Add missing message statuses fix(sdk): Add missing message status index Jul 11, 2023
@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Merging #6254 (a666c4f) into develop (d80c145) will decrease coverage by 1.36%.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #6254      +/-   ##
===========================================
- Coverage    44.31%   42.95%   -1.36%     
===========================================
  Files          436      332     -104     
  Lines        29025    25901    -3124     
  Branches       709      372     -337     
===========================================
- Hits         12863    11127    -1736     
+ Misses       15085    13773    -1312     
+ Partials      1077     1001      -76     
Flag Coverage Δ
bedrock-go-tests 43.08% <ø> (-0.03%) ⬇️
cannon-go-tests ?
common-ts-tests ?
contracts-bedrock-tests ?
core-utils-tests ?
fault-detector-tests 26.95% <ø> (ø)
sdk-next-tests 42.45% <0.00%> (-0.03%) ⬇️
sdk-tests 42.45% <0.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/sdk/src/cross-chain-messenger.ts 51.82% <0.00%> (-0.10%) ⬇️

... and 106 files with indirect coverage changes

Copy link
Member

@nickbalestra nickbalestra left a comment

Choose a reason for hiding this comment

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

Left a non-blocking comment on the comment that was not to clear to me to read and understand and I imagine I will not understand it in few weeks if i get back to it :D

@roninjin10 roninjin10 merged commit 8a3ec2b into develop Jul 11, 2023
10 checks passed
@roninjin10 roninjin10 deleted the 07-11-fix_sdk_Add_missing_message_statuses branch July 11, 2023 22:01
This was referenced Jul 18, 2023
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.

3 participants