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(backend): ♻️ substantial refactor of generic usage in the codebase #22

Merged
merged 13 commits into from
Mar 23, 2024

Conversation

Trantorian1
Copy link
Collaborator

Pull Request type

Please add the labels corresponding to the type of changes your PR introduces:

  • Refactoring (no functional changes, no API changes)

What is the current behavior?

Deoxys in its current state overly relies on generic types to satisfy Substrate function requirements. This is not necessary since we know the type of our blocks at compile time. This PR aims to fix this issue.

What is the new behavior?

  • Significantly reduced reliance on sp_runtime::traits::Block as BlockT and sp_runtime::traits::Block::Hash as HashT throughout the codebase.
  • Set up fixed types for block, block header and block hash in the form of DBlockT, DHeaderT and DHashT. These serve as a standard for the types deoxys uses throughout the codebase and allow for easier reasoning as to the types we are working with. This is especially the case in l2.rs which can undoubtedly be further refactored as a result.
  • Converted mc_db::Backend to mc_db::DeoxysBackend and made it a singleton. Note that this means that backend databases now have to be accessed via static methods. Take a look at the code in crates/client/db/src/lib.rs for more information.

Does this introduce a breaking change?

No.

Other information

⚠️ Please submit any feedback regarding potential breaking changes, especially in regards to rpc methods, as this is where the DeoxysBackend has the most impact.

Trantorian added 6 commits March 20, 2024 17:28
New code uses explicit `madara_runtime::opaque::Block` as Substrate block type
with `H256` hash. This makes it easier to reason about types in the codebase,
since in any case we are only dealing with one block time which should be cleary
stated.

> **TODO**: futher refactor needed
@Trantorian1 Trantorian1 added the enhancement New feature or request label Mar 21, 2024
@Trantorian1 Trantorian1 self-assigned this Mar 21, 2024
@Trantorian1 Trantorian1 marked this pull request as ready for review March 22, 2024 14:40
Trantorian added 2 commits March 22, 2024 14:56
Also added some new `check` and `ready` options to `./scripts/deoxys` to make
these easier to detect.
@Trantorian1 Trantorian1 requested review from Tbelleng and removed request for jbcaron March 22, 2024 15:26
Copy link
Contributor

@Tbelleng Tbelleng left a comment

Choose a reason for hiding this comment

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

Backend refacto seems good to me 👍

@antiyro antiyro merged commit 9d8b269 into madara-alliance:main Mar 23, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants