Skip to content

feat: correct conf_remaining calculation#2541

Merged
mergify[bot] merged 2 commits intoethereum-optimism:developfrom
cfromknecht:teleportr-fixes
May 5, 2022
Merged

feat: correct conf_remaining calculation#2541
mergify[bot] merged 2 commits intoethereum-optimism:developfrom
cfromknecht:teleportr-fixes

Conversation

@cfromknecht
Copy link
Contributor

Subtaction was being performed in the wrong order, causing underflow.

Metadata

  • Fixes ENG-2191

@changeset-bot
Copy link

changeset-bot bot commented May 5, 2022

🦋 Changeset detected

Latest commit: 7fb8624

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

This PR includes changesets to release 1 package
Name Type
@eth-optimism/teleportr 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

@mergify
Copy link
Contributor

mergify bot commented May 5, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

@mergify mergify bot requested review from Inphi and tuxcanfly May 5, 2022 16:47
Copy link
Collaborator

@mslipper mslipper left a comment

Choose a reason for hiding this comment

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

LGTM, just add a changeset.

Subtaction was being performed in the wrong order, causing underflow.
@mergify
Copy link
Contributor

mergify bot commented May 5, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify mergify bot requested a review from Inphi May 5, 2022 17:28
@mergify mergify bot merged commit 44c293d into ethereum-optimism:develop May 5, 2022
@mergify
Copy link
Contributor

mergify bot commented May 5, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@cfromknecht cfromknecht deleted the teleportr-fixes branch May 5, 2022 17:28
@cfromknecht cfromknecht restored the teleportr-fixes branch May 5, 2022 17:28
@cfromknecht cfromknecht deleted the teleportr-fixes branch May 5, 2022 17:28
@cfromknecht cfromknecht restored the teleportr-fixes branch May 5, 2022 17:28
Copy link
Contributor

@roninjin10 roninjin10 left a comment

Choose a reason for hiding this comment

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

damn_good_stuff_sir

confsRemaining = blockNumber + 1 -
(teleport.Deposit.BlockNumber + s.numConfirmations)
confsRemaining = teleport.Deposit.BlockNumber + s.numConfirmations -
(blockNumber + 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wierd thing is I was reading this code and in my head it looked correct to me at the time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

haha well it looked correct to me as well when i wrote the first time 😅

This was referenced May 10, 2022
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.

4 participants