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

refactor/pro-1160/remove-side-side-map #4489

Merged
merged 15 commits into from
Feb 21, 2024

Conversation

Janislav
Copy link
Contributor

@Janislav Janislav commented Feb 6, 2024

Pull Request

Closes: PRO-1160

Checklist

Please conduct a thorough self-review before opening the PR.

  • I am confident that the code works.
  • I have updated documentation where appropriate.

Summary

Please include a succinct description of the purpose and content of the PR. What problem does it solve, and how? Link issues, discussions, other PRs, and anything else that will help the reviewer.

Copy link

codecov bot commented Feb 6, 2024

Codecov Report

Attention: 81 lines in your changes are missing coverage. Please review.

Comparison is base (cb12b67) 73% compared to head (27f29dc) 72%.
Report is 5 commits behind head on main.

❗ Current head 27f29dc differs from pull request most recent head f9e2297. Consider uploading reports for the commit f9e2297 to get more accurate results

Files Patch % Lines
state-chain/amm/src/lib.rs 42% 35 Missing ⚠️
state-chain/pallets/cf-pools/src/lib.rs 57% 25 Missing and 1 partial ⚠️
state-chain/amm/src/common.rs 85% 5 Missing and 2 partials ⚠️
state-chain/amm/src/range_orders.rs 80% 4 Missing ⚠️
state-chain/custom-rpc/src/lib.rs 0% 4 Missing ⚠️
state-chain/runtime/src/lib.rs 0% 0 Missing and 3 partials ⚠️
api/lib/src/lp.rs 0% 2 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##            main   #4489    +/-   ##
======================================
- Coverage     73%     72%    -0%     
======================================
  Files        401     399     -2     
  Lines      68209   67725   -484     
  Branches   68209   67725   -484     
======================================
- Hits       49617   49096   -521     
- Misses     15964   15972     +8     
- Partials    2628    2657    +29     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Janislav
Copy link
Contributor Author

Janislav commented Feb 7, 2024

@AlastairHolmes for now I kept the term AssetsMap because I actually think it's not the worst name. I changed the Assets enum to Pairs. Maybe because of that, the terminology PoolPair could make sense?

@Janislav Janislav marked this pull request as ready for review February 7, 2024 10:21
@Janislav Janislav requested review from AlastairHolmes and removed request for dandanlen February 7, 2024 10:21
@AlastairHolmes
Copy link
Contributor

Can we also rename: "to_sold_side" to "to_sold_pair"

@AlastairHolmes
Copy link
Contributor

@AlastairHolmes for now I kept the term AssetsMap because I actually think it's not the worst name. I changed the Assets enum to Pairs. Maybe because of that, the terminology PoolPair could make sense?

I like Pair/PoolPair, but wrt AssetsMap, I think its nice to keep consistently with the Map naming. So maybe (Pair, PairMap) / (Pairs, PairsMap) / (PoolPair, PoolPairMap) / (PoolPairs, PoolPairsMap)?

@AlastairHolmes
Copy link
Contributor

Also we should rename "enum Order" to "Side" in this PR too.

@AlastairHolmes
Copy link
Contributor

@AlastairHolmes for now I kept the term AssetsMap because I actually think it's not the worst name. I changed the Assets enum to Pairs. Maybe because of that, the terminology PoolPair could make sense?

I like Pair/PoolPair, but wrt AssetsMap, I think its nice to keep consistently with the Map naming. So maybe (Pair, PairMap) / (Pairs, PairsMap) / (PoolPair, PoolPairMap) / (PoolPairs, PoolPairsMap)?

Lets stick with the Plural "Pairs"/"PoolPairs", as I want to use "PoolPair" for something else.

@Janislav Janislav merged commit 50adebf into main Feb 21, 2024
41 of 42 checks passed
@Janislav Janislav deleted the refactor/pro-1160/remove-side-side-map branch February 21, 2024 12:10
syan095 added a commit that referenced this pull request Feb 26, 2024
…ero-liquidity

* origin/main: (28 commits)
  feat(custom-rpc): add broker info [WEB-925] (#4560)
  chore: upgrade solana version (#4567)
  fix: continuous adapter (PRO-684) (#4503)
  fix: Wait for ThresholdSignature success before switching to NewKeysActivated (#4534)
  chore: remove un-needed serde derives from EncodedAddress (#4565)
  fix: more lenient max deposit fee in bouncer test (#4564)
  chore: build persa bins instead of fetch (#4554)
  feat: deploy L2 contracts upon localnet startup and send L2 TXs 📑 (#4558)
  feat: debug logs on runtime upgrade test (#4556)
  feature/PRO-1038/pool-fee-rpc (#4459)
  chore: update bootnodes in chainspec ✨ (#4456)
  fix: allow test coverage to run (#4555)
  refactor/pro-1160/remove-side-side-map (#4489)
  fix: activate missing migrations (#4550)
  chore: remove cf-build (#4551)
  feat: extensible multi-key rotator (#4546)
  fix: ensure channel open fee can be paid in benchmarks (#4544)
  feat: bump substrate deps to polkadot-sdk 1.6 (#4504)
  fix: upgrade-test should restart the chainflip-nodes on an incompatible upgrade (#4490)
  test: add small v3 migration test (#4543)
  ...

# Conflicts:
#	state-chain/pallets/cf-pools/src/tests.rs
syan095 added a commit that referenced this pull request Feb 26, 2024
…-price

* origin/main:
  feat(custom-rpc): add broker info [WEB-925] (#4560)
  chore: upgrade solana version (#4567)
  fix: continuous adapter (PRO-684) (#4503)
  fix: Wait for ThresholdSignature success before switching to NewKeysActivated (#4534)
  chore: remove un-needed serde derives from EncodedAddress (#4565)
  fix: more lenient max deposit fee in bouncer test (#4564)
  chore: build persa bins instead of fetch (#4554)
  feat: deploy L2 contracts upon localnet startup and send L2 TXs 📑 (#4558)
  feat: debug logs on runtime upgrade test (#4556)
  feature/PRO-1038/pool-fee-rpc (#4459)
  chore: update bootnodes in chainspec ✨ (#4456)
  fix: allow test coverage to run (#4555)
  refactor/pro-1160/remove-side-side-map (#4489)
  fix: activate missing migrations (#4550)
  chore: remove cf-build (#4551)

# Conflicts:
#	state-chain/pallets/cf-pools/src/lib.rs
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.

2 participants