Skip to content

fix: no default accounts#306

Merged
weichweich merged 8 commits intodevelopfrom
aw-no-default-accoutns
Dec 17, 2021
Merged

fix: no default accounts#306
weichweich merged 8 commits intodevelopfrom
aw-no-default-accoutns

Conversation

@weichweich
Copy link
Copy Markdown
Contributor

@weichweich weichweich commented Dec 13, 2021

fixes KILTProtocol/ticket#1715

This should not change anything. But just to make sure that these potentially problematic lines are not copied and used somewhere else, I would change it.

Checklist:

  • I have verified that the code works
    • No panics! (checked arithmetic ops, no indexing array[3] use get(3), ...)
  • I have verified that the code is easy to understand
    • If not, I have left a well-balanced amount of inline comments
  • This PR does not introduce new custom types
  • I have left the code in a better state
  • I have documented the changes (where applicable)

@weichweich weichweich requested review from ntn-x2 and wischli December 14, 2021 09:07
Copy link
Copy Markdown
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

I would like to move the pallet ids to the constants and import them from there.

Comment thread primitives/src/constants.rs
Comment thread pallets/kilt-launch/src/mock.rs Outdated
Copy link
Copy Markdown
Contributor

@ntn-x2 ntn-x2 left a comment

Choose a reason for hiding this comment

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

LGMT! I did not check the crowdloan pallet as I guess we need to merge master back into develop and develop back into this branch.

Comment thread primitives/src/lib.rs Outdated
pub type SlowAdjustingFeeUpdate<R> =
TargetedFeeAdjustment<R, TargetBlockFullness, AdjustmentVariable, MinimumMultiplier>;

/// Unique identifiers for pallets.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

W: I would move the pallet_id module into its own file. I know it does not have a lot of information, but it is easier to find. This lib.rs file feels like a big pot of stuff.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@weichweich weichweich enabled auto-merge (squash) December 16, 2021 12:28
@weichweich weichweich disabled auto-merge December 16, 2021 12:30
Copy link
Copy Markdown
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

LGTM!

@weichweich weichweich merged commit 07c412c into develop Dec 17, 2021
@weichweich weichweich deleted the aw-no-default-accoutns branch December 17, 2021 09:54
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