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

(wip) refactor: action enum #240

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

swissarmytowel
Copy link
Collaborator

Summary

Removes ActionType enum and all relevant code in favour of defining in every margin engine repo.

Checklist

Ensure you completed all of the steps below before submitting your pull request:

  • Add natspec for all functions / parameters
  • Ran forge snapshot
  • Ran forge fmt
  • Ran forge test
  • Triage Slither issues, and post uncertain ones in the PR

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

Gas usage comparison:

Suite Case Gas Diff Percentage Change
BaseDebitSpreadEngineFlow testAddCollateralMoveBalance ♻️ -249 -0.37%
testAddLongShouldMoveToken ♻️ -440 -0.43%
testBurnActionShouldBurnOption ♻️ -352 -0.67%
testCannotAddCollatFromOthers ♻️ -254 -1.63%
testCannotAddLongFromOthers ♻️ -227 -1.41%
testCannotBurnFromOthers ♻️ -219 -1.28%
testCannotMergeWithSameStrike ♻️ -122 -0.78%
testCannotRemoveMoreThanEngineHas ♻️ -474 -0.65%
testMergeActionShouldBurnToken ♻️ -272 -0.51%
testMintActionShouldMintOption ♻️ -214 -0.36%
testRemoveCollateralMoveBalance ♻️ -399 -0.68%
testRemoveLongShouldPullToken ♻️ -455 -0.58%
testSettlementShouldEmitEvent ♻️ -158 -0.63%
testSplitActionShouldMintToken ♻️ -236 -0.39%
OptionTransferableTest testCanTransferCollat ♻️ -648 -0.61%
testCannotTransferShortIfEndingUnderwater ♻️ -647 -0.58%
testMintOptionToOtherAccount ♻️ -647 -0.40%
testTransferLong ♻️ -635 -0.56%
testTransferShort ♻️ -646 -0.47%

@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #240 (3340e99) into master (1d18f28) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #240   +/-   ##
=======================================
  Coverage   91.45%   91.45%           
=======================================
  Files          14       14           
  Lines         351      351           
  Branches       73       73           
=======================================
  Hits          321      321           
  Misses         20       20           
  Partials       10       10           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@swissarmytowel swissarmytowel changed the base branch from master to feature/burn-in-account October 6, 2023 12:59
Base automatically changed from feature/burn-in-account to master October 12, 2023 16:41
@antoncoding
Copy link
Member

Please rebase this PR before merging!

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