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.

Sign up for GitHub

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

Jump to bottom

NODE-79, bifrost pallets improvements #34

Merged
merged 44 commits into from
Jan 10, 2024
Merged

NODE-79, bifrost pallets improvements #34

merged 44 commits into from
Jan 10, 2024

Conversation

alstjd0921
Copy link
Member

@alstjd0921 alstjd0921 commented Jan 4, 2024 •
edited by dnjscksdn98
Loading

Description

  • Overall, better expressions applied to the parts that could be used
  • Replace OrderedSet with BTreeMap & BTreeSet
  • Remove unnecessary clone()s. or partially clone.
  • Remove redundant codes
  • Remove dead code
  • Migrate to new standard storage version
  • Remove possibly panic points as much as possible
  • Remove unnecessary path prefixes
  • Fix wrong revert message in precompiles

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Something else (simple changes that are not related to existing functionality or others)

Checklist

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation.
  • I have made new test codes regarding to my changes.
  • I have no personal secrets or credentials described on my changes.
  • I have run cargo-clippy and linted my code.
  • My changes generate no new warnings.
  • My changes passed the existing test codes.
  • My changes are able to compile.

@alstjd0921
NODE-79, feature: migrate to standard storage version for bfc-staking
3cd9812
@alstjd0921
NODE-79, feature: migrate to use new standard storage version for `bf…
eb7ed0c 
…c-offences`, `bfc-utility`, `relay-manager`.
@alstjd0921
NODE-79, refactor: replace OrderedSet with BTreeMap & BTreeSet
b2f2cd7
@alstjd0921
NODE-79, refactor: calculate translate weight info
13a2dfa
@alstjd0921
NODE-79, refactor: Remove redundant code
d41f9a2
@alstjd0921
NODE-79, refactor: Remove panic point in bfc-staking as much as pos…
75c4c89 
…sible
@alstjd0921
NODE-79, refactor: use btree set in SelectedCandidates, `SelectedFu…
5f67a96 
…llCandidates`, `SelectedBasicCandidates`, `CachedSelectedCandidates`
@alstjd0921
NODE-79, refactor: remove unnecessary path prefix
067e860
@alstjd0921
NODE-79, refactor: remove unnecessary reverse
85eec52
@alstjd0921
NODE-79, refactor: migrate CandidatePool to use BoundedBTreeMap
133785e
@alstjd0921
NODE-79, refactor: kill storage MinTotalSelected
fa07dd0
@alstjd0921
NODE-79, refactor: remove unnecessary path prefix
1ef7636
@alstjd0921
NODE-79, fix: typo
8200f9a
@alstjd0921
NODE-79, refactor: use iter_mut
220be67
@alstjd0921
NODE-79, chore: remove deadcode
10084d9
@alstjd0921
NODE-79, refactor: use mutate
6217647
@alstjd0921
NODE-79, refactor: clone in small range
98b6a14
@alstjd0921
NODE-79, refactor: no need to be complicate
b546eff
@alstjd0921
NODE-79, refactor: use getter
643186b
@alstjd0921
NODE-79, fix: should use take instead get
cfede72
@alstjd0921
NODE-79, refactor: remove dead code
bfd90f8
@alstjd0921
NODE-79, refactor: no need to be complicated
8e78445
@alstjd0921
NODE-79, refactor: use getter
ecc8739
@alstjd0921
NODE-79, refactor: relay-manager
1e67f98
@alstjd0921
NODE-79, refactor: .
ae837c7
@alstjd0921
NODE-79, refactor: remove redundant
f00108a
@alstjd0921
NODE-79, fix: revert message
434a5ef
@alstjd0921
NODE-79, refactor: bfc-staking
d4fa764
@alstjd0921
NODE-79, refactor: relay-manager
6e2a42a
@alstjd0921
NODE-79, refactor: move under util methods
5c771ad
@alstjd0921 alstjd0921 self-assigned this Jan 4, 2024
@alstjd0921 alstjd0921 changed the title Node 79 pallet storage type improvements pallet storage type improvements Jan 4, 2024
@alstjd0921 alstjd0921 changed the title pallet storage type improvements NODE-79, bifrost pallets improvements Jan 5, 2024
@alstjd0921 alstjd0921 marked this pull request as ready for review January 5, 2024 09:09
@dnjscksdn98
Copy link
Member

혹시 다음 사항들 테스트 되었을까요?

  • 런타임 업그레이드 (마이그레이션 포함)
  • 기존 기능 테스트
  • 테스트 코드 (추가될 테스트는 없는지)

dnjscksdn98
dnjscksdn98 reviewed Jan 8, 2024
View reviewed changes
Copy link
Member

@dnjscksdn98 dnjscksdn98 left a comment

Choose a reason for hiding this comment

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

코드 업데이트 양도 많고 꼼꼼하게 보기 위해 몇번에 걸쳐서 리뷰하는게 좋을것 같네요.
우선 1차 리뷰되었고 내일 더 보겠습니다!

pallets/bfc-offences/src/migrations.rs Show resolved Hide resolved
pallets/bfc-offences/src/pallet/impls.rs Show resolved Hide resolved
pallets/bfc-offences/src/pallet/mod.rs Show resolved Hide resolved
pallets/bfc-offences/src/pallet/mod.rs Show resolved Hide resolved
pallets/bfc-offences/src/pallet/mod.rs Outdated Show resolved Hide resolved
pallets/bfc-staking/src/pallet/mod.rs Show resolved Hide resolved
pallets/relay-manager/src/pallet/impls.rs Outdated Show resolved Hide resolved
dnjscksdn98
dnjscksdn98 reviewed Jan 9, 2024
View reviewed changes
Copy link
Member

@dnjscksdn98 dnjscksdn98 left a comment

Choose a reason for hiding this comment

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

OrderedSet이 더이상 사용되지 않는다면 set.rs 파일도 제거해도 될것 같네요

pallets/bfc-staking/src/migrations.rs Outdated Show resolved Hide resolved
pallets/bfc-staking/src/migrations.rs Outdated Show resolved Hide resolved
pallets/bfc-offences/src/pallet/mod.rs Show resolved Hide resolved
tests/tests/pallets/test_relay_manager.ts

// check `CachedInitialSelectedRelayers`
const rawCachedInitialRelayers: any = await context.polkadotApi.query.relayManager.cachedInitialSelectedRelayers();
const cachedInitialRelayers = rawCachedInitialRelayers.toJSON();
expect(cachedInitialRelayers[0][1].length).equal(1);
expect(cachedInitialRelayers[0][1]).include(alithRelayer.address);
// expect(cachedInitialRelayers[0][1].length).equal(1);
Copy link
Member

Choose a reason for hiding this comment

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

expect(cachedInitialRelayers[currentRound].length).equal(1);

이거는 테스트가 안되나요?

@alstjd0921 alstjd0921 changed the base branch from main to pre-v1.3.0 January 10, 2024 05:11
@alstjd0921 alstjd0921 merged commit e2b3afa into pre-v1.3.0 Jan 10, 2024
dnjscksdn98 pushed a commit that referenced this pull request Jan 12, 2024
@alstjd0921 @dnjscksdn98
NODE-79, bifrost pallets improvements (#34)
f829d6b 
* NODE-79, feature: migrate to standard storage version for bfc-staking

* NODE-79, feature: migrate to use new standard storage version for `bfc-offences`, `bfc-utility`, `relay-manager`.

* NODE-79, refactor: replace `OrderedSet` with `BTreeMap` & `BTreeSet`

* NODE-79, refactor: calculate translate weight info

* NODE-79, refactor: Remove redundant code

* NODE-79, refactor: Remove panic point in `bfc-staking` as much as possible

* NODE-79, refactor: use btree set in `SelectedCandidates`, `SelectedFullCandidates`, `SelectedBasicCandidates`, `CachedSelectedCandidates`

* NODE-79, refactor: remove unnecessary path prefix

* NODE-79, refactor: remove unnecessary reverse

* NODE-79, refactor: migrate `CandidatePool` to use BoundedBTreeMap

* NODE-79, refactor: kill storage `MinTotalSelected`

* NODE-79, refactor: remove unnecessary path prefix

* NODE-79, fix: typo

* NODE-79, refactor: use iter_mut

* NODE-79, chore: remove deadcode

* NODE-79, refactor: use mutate

* NODE-79, refactor: clone in small range

* NODE-79, refactor: no need to be complicate

* NODE-79, refactor: use getter

* NODE-79, fix: should use take instead get

* NODE-79, refactor: remove dead code

* NODE-79, refactor: no need to be complicated

* NODE-79, refactor: use getter

* NODE-79, refactor: `relay-manager`

* NODE-79, refactor: .

* NODE-79, refactor: remove redundant

* NODE-79, fix: revert message

* NODE-79, refactor: `bfc-staking`

* NODE-79, refactor: `relay-manager`

* NODE-79, refactor: move under util methods

* NODE-79, refactor: impl From<Offence> for EvmOffence

* NODE-79, refactor: reduce getter call

* NODE-79, refactor: remove unnecessary clone

* NODE-79, refactor: `Nominator.initial_nominations` type to BTreeMap

* NODE-79, refactor: .

* NODE-79, chore: remove dead code

* NODE-79, fix:

* NODE-79, fix:

* NODE-79, test: fit into new storage format

* NODE-79, fix: modify if exists else insert

* NODE-79, refactor: remove dead code

* NODE-79, fix: condition check on OnRuntimeUpgrade

* NODE-79, chore: comment

* NODE-79, fix: remove duplicate translate in migrations
dnjscksdn98 pushed a commit that referenced this pull request Jan 12, 2024
@alstjd0921 @dnjscksdn98
NODE-79, bifrost pallets improvements (#34)
d230dde 
* NODE-79, feature: migrate to standard storage version for bfc-staking

* NODE-79, feature: migrate to use new standard storage version for `bfc-offences`, `bfc-utility`, `relay-manager`.

* NODE-79, refactor: replace `OrderedSet` with `BTreeMap` & `BTreeSet`

* NODE-79, refactor: calculate translate weight info

* NODE-79, refactor: Remove redundant code

* NODE-79, refactor: Remove panic point in `bfc-staking` as much as possible

* NODE-79, refactor: use btree set in `SelectedCandidates`, `SelectedFullCandidates`, `SelectedBasicCandidates`, `CachedSelectedCandidates`

* NODE-79, refactor: remove unnecessary path prefix

* NODE-79, refactor: remove unnecessary reverse

* NODE-79, refactor: migrate `CandidatePool` to use BoundedBTreeMap

* NODE-79, refactor: kill storage `MinTotalSelected`

* NODE-79, refactor: remove unnecessary path prefix

* NODE-79, fix: typo

* NODE-79, refactor: use iter_mut

* NODE-79, chore: remove deadcode

* NODE-79, refactor: use mutate

* NODE-79, refactor: clone in small range

* NODE-79, refactor: no need to be complicate

* NODE-79, refactor: use getter

* NODE-79, fix: should use take instead get

* NODE-79, refactor: remove dead code

* NODE-79, refactor: no need to be complicated

* NODE-79, refactor: use getter

* NODE-79, refactor: `relay-manager`

* NODE-79, refactor: .

* NODE-79, refactor: remove redundant

* NODE-79, fix: revert message

* NODE-79, refactor: `bfc-staking`

* NODE-79, refactor: `relay-manager`

* NODE-79, refactor: move under util methods

* NODE-79, refactor: impl From<Offence> for EvmOffence

* NODE-79, refactor: reduce getter call

* NODE-79, refactor: remove unnecessary clone

* NODE-79, refactor: `Nominator.initial_nominations` type to BTreeMap

* NODE-79, refactor: .

* NODE-79, chore: remove dead code

* NODE-79, fix:

* NODE-79, fix:

* NODE-79, test: fit into new storage format

* NODE-79, fix: modify if exists else insert

* NODE-79, refactor: remove dead code

* NODE-79, fix: condition check on OnRuntimeUpgrade

* NODE-79, chore: comment

* NODE-79, fix: remove duplicate translate in migrations
@alstjd0921 alstjd0921 deleted the NODE-79-pallet-storage-type-improvements branch January 16, 2024 06:28
dnjscksdn98 added a commit that referenced this pull request Feb 6, 2024
@dnjscksdn98 @alstjd0921
Release v1.3.0 (#39)
5e6b7f7 
* NODE-79, bifrost pallets improvements (#34)

* NODE-79, feature: migrate to standard storage version for bfc-staking

* NODE-79, feature: migrate to use new standard storage version for `bfc-offences`, `bfc-utility`, `relay-manager`.

* NODE-79, refactor: replace `OrderedSet` with `BTreeMap` & `BTreeSet`

* NODE-79, refactor: calculate translate weight info

* NODE-79, refactor: Remove redundant code

* NODE-79, refactor: Remove panic point in `bfc-staking` as much as possible

* NODE-79, refactor: use btree set in `SelectedCandidates`, `SelectedFullCandidates`, `SelectedBasicCandidates`, `CachedSelectedCandidates`

* NODE-79, refactor: remove unnecessary path prefix

* NODE-79, refactor: remove unnecessary reverse

* NODE-79, refactor: migrate `CandidatePool` to use BoundedBTreeMap

* NODE-79, refactor: kill storage `MinTotalSelected`

* NODE-79, refactor: remove unnecessary path prefix

* NODE-79, fix: typo

* NODE-79, refactor: use iter_mut

* NODE-79, chore: remove deadcode

* NODE-79, refactor: use mutate

* NODE-79, refactor: clone in small range

* NODE-79, refactor: no need to be complicate

* NODE-79, refactor: use getter

* NODE-79, fix: should use take instead get

* NODE-79, refactor: remove dead code

* NODE-79, refactor: no need to be complicated

* NODE-79, refactor: use getter

* NODE-79, refactor: `relay-manager`

* NODE-79, refactor: .

* NODE-79, refactor: remove redundant

* NODE-79, fix: revert message

* NODE-79, refactor: `bfc-staking`

* NODE-79, refactor: `relay-manager`

* NODE-79, refactor: move under util methods

* NODE-79, refactor: impl From<Offence> for EvmOffence

* NODE-79, refactor: reduce getter call

* NODE-79, refactor: remove unnecessary clone

* NODE-79, refactor: `Nominator.initial_nominations` type to BTreeMap

* NODE-79, refactor: .

* NODE-79, chore: remove dead code

* NODE-79, fix:

* NODE-79, fix:

* NODE-79, test: fit into new storage format

* NODE-79, fix: modify if exists else insert

* NODE-79, refactor: remove dead code

* NODE-79, fix: condition check on OnRuntimeUpgrade

* NODE-79, chore: comment

* NODE-79, fix: remove duplicate translate in migrations

* NODE-65, deps: polkadot sdk migration (#36)

* NODE-64, deps: upgrade to bifrost-polkadot-v1.3.0

* NODE-64, chore: use

* NODE-64, chore: remove precompile utils

* NODE-64, chore: replace to

* NODE-64, chore: move  to

* NODE-65, chore: apply block number prelude to precompiles

* NODE-65, chore: add hold/freeze reasons to

* NODE-65, chore: remove hold identifier from

* NODE-65, chore: add  to

* NODE-65, chore: add AllowMultipleBlocksPerSlot to pallet_aura

* NODE-65, chore: add new types to pallet_treasury

* NODE-65, chore: make pallet_preimage use Consideration

* NODE-65, chore: add missing generic type to GenesisConfig

* NODE-65, chore: update pallet_preimage

* NODE-65, chore: add IdentityInformation to pallet_identity

* NODE-65, chore: remove MaxPeerDataEncodingSize from pallet_im_online

* NODE-65, chore: add SuicideQuickClearLimit to pallet_evm

* NODE-65, feat: implement GenesisBuilder runtime api

* NODE-65, chore: rename Index to Nonce

* NODE-65, chore: specify txpool feature to fc_rpc

* NODE-65, fix: pending runtime api for pending state

* NODE-65, chore: Removal of execution strategies

* NODE-65, chore: Set StateBackend::Transaction to PrefixedMemoryDB

* NODE-65, chore: Modular block request handler

* NODE-65, chore: implement eth_call state override

* NODE-65, fix: resolve service.rs compile errors

* NODE-65, fix: replace to RuntimeGenesisConfig in chain_spec

* NODE-65, chore: add missing clone()

* NODE-65, chore: remove native runtime version api

* NODE-65, fix: replace to fp-account

* NODE-65, fix: remove RuntimeFreezeReason and sort imports

* NODE-65, chore: remove unneccessary self imports

* NODE-65, fix: rollback cherry-picked EthereumRuntimeRPCApi changes

* NODE-65, fix: add missing evm features

* NODE-65, deps: release v1.3.0

* NODE-65, fix: remove unused params

* NODE-65, fix: use correct crate features

* NODE-65, fix: remove std feature on pallet GenesisConfig Default impl

* NODE-65, test: replace deprecated storage

* NODE-65, test: estimated gas should be correct

* NODE-65, test: split runtime tests

* NODE-65, test: check balance consistancy

* NODE-65, test: check balance consistancy (2)

* NODE-65, feat: Only use substrate-wasm-builder when std feature is enabled

* NODE-65, deps: update Cargo.lock

* NODE-65, fix: resolve imports

* NODE-65, fix: update runtime tests

* NODE-65, fix: handle runtime test failures

* NODE-65, deps: update node packages

* NODE-65, deps: update Cargo.lock

* NODE-65, fix: remove test config

* NODE-84, chore: bifrost-frontier/pull/12 (#37)

* NODE-65, test: bifrost-frontier/pull/12 test code

---------

Co-authored-by: Min-seong Kwon <kwonarseus@gmail.com>

* NODE-78, fix: delayed relayer address update (#38)

* NODE-64, deps: upgrade to bifrost-polkadot-v1.3.0

* NODE-65, fix: pending runtime api for pending state

* NODE-65, chore: implement eth_call state override

* NODE-65, test: split runtime tests

* NODE-65, fix: update runtime tests

* NODE-65, fix: handle runtime test failures

* NODE-65, fix: remove test config

* NODE-78, feat: use delayed relayer sets

* NODE-78, feat: handle relayer sets on round update

* NODE-78, fix: add verifications on scheduling leaves

* NODE-78, test: add basic set relayer tests

* NODE-78, test: add advanced tests

* NODE-78, feat: add constraints to set_controller

* NODE-78, feat: add constraints to set_validator_commission

* NODE-78, chore: expect replacement will always succeed

* NODE-78, chore: add missing string docs

* NODE-78, fix: round update must be priorly set

* NODE-78, chore: add missing string docs

* NODE-78, fix: remove redundant state checks

* deps: increase runtime version

* test: update live runtime tests

---------

Co-authored-by: Min-seong Kwon <kwonarseus@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dnjscksdn98 dnjscksdn98 dnjscksdn98 approved these changes

Assignees

@alstjd0921 alstjd0921

Labels
chore
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alstjd0921 @dnjscksdn98