fix: resolve review comments #454
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
unclezoro
reviewed
Dec 26, 2023
contracts/TokenHub.sol
Outdated
| @@ -167,7 +167,7 @@ contract TokenHub is ITokenHub, System, IParamSubscriber, IApplication, ISystemR | |||
| return actualAmount; | |||
| } | |||
|
|
|||
| function claimRewardsforFinality(address payable, uint256) onlyInit onlyRelayerIncentivize external override returns(uint256) { | |||
| function claimLargeRewards(address payable, uint256) onlyInit onlyRelayerIncentivize external override returns(uint256) { | |||
| revert("CLAIM_REWARDS_FOR_FINALITY_NOT_ALLOWED"); | |||
There was a problem hiding this comment.
CLAIM_REWARDS_FOR_FINALITY_NOT_ALLOWED?
There was a problem hiding this comment.
After review and analysis by merge confirmation as completed
pythonberg1997
force-pushed
the
bc-fusion
branch
from
db3827b to
75c96c7
Compare
buddh0
approved these changes
Dec 26, 2023
|
LGTM except CLAIM_REWARDS_FOR_FINALITY_NOT_ALLOWED |
unclezoro
reviewed
Dec 26, 2023
pythonberg1997
force-pushed
the
bc-fusion
branch
2 times, most recently
from
97111ff to
ed2993f
Compare
unclezoro
approved these changes
Dec 26, 2023
pythonberg1997
force-pushed
the
bc-fusion
branch
2 times, most recently
from
8650392 to
b896021
Compare
pythonberg1997
force-pushed
the
bc-fusion
branch
from
b896021 to
64c57dc
Compare
unclezoro
reviewed
Jan 3, 2024
contracts/BC_fusion/StakeHub.sol
Outdated
| if (isLast) { | ||
| // 0x08 is the staking channel id. If this channel is closed, then BC-fusion is finished and we should keep the last eligible validator here | ||
| if ( | ||
| !ICrossChain(CROSS_CHAIN_CONTRACT_ADDR).registeredContractChannelMap(VALIDATOR_CONTRACT_ADDR, 0x08) |
There was a problem hiding this comment.
We should put isLast before the channel check, because in most cases isLast will be false, then channel check will be unnecessary, it can save gas.
unclezoro
approved these changes
Jan 3, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 26, 2024
* feat: implement BEP294 and BEP297 (#404) * feat: implement BEP294 * feat: add BSCGovernor and GovToken contracts for bc-fusion (#403) * feat: implement BEP294 --------- Co-authored-by: Ethan <cosinlinker@gmail.com> * feat: implement BEP299 (#391) * chore: add init lock amount (#425) * chore: add init lock amount * add annotations * more comment --------- Co-authored-by: zjubfd <296179868@qq.com> * chore: add redelegate fee (#426) * feat: add comments and pause for governor (#428) * chore: fix typo and update scripts (#429) * feat: add apy and reward record of validators (#430) * feat: add apy and reward record of validators * fix review comments * feat: add ci to bc-fusion contracts and lint code (#431) * feat: add ci and add lint check script * chore: lint code in BC_fusion * chore: add bc-fusion branch to ci * fix: test error in SlashIndicator.t.sol * feat: add python script (#433) * chore: fix typo * feat: add python script * fix review comments * fix review comments * chore: fix bugs and add annotation for error msgs * fix: init params error in BSCGovernor (#439) * chore: update storages and generate.py (#440) * chore: disable editing moniker (#442) * fix: update slash logic to avoid malicious slash (#444) * chore: add check for moniker and update script (#445) * chore: ensure burnRatio plus systemRewardRatio not greater than 100% (#446) * fix: ensure validator incoming will be clear after distribution (#447) * chore: add view function and update annotations (#448) * fix: recover sig failed in token recover portal (#451) * fix: token recover contract recover approval sig * docs: update abi * fix: add generate option to tokenhub * fix: lint * fix: `felonySlashScope` not initialized (#452) * fix: resolve review comments (#454) * fix: resolve review comments * add check to `_jailValidator` * fix lint issue * fix review comments * fix: audit report (#455) * feat: add whenNotPaused for queue in BSCGovernor (#456) * feat: add whenNotPaused for queue in BSCGovernor * chores: lint code * feat: add comments * fix: lint * feat: add `handleSynPackage` to `StakeHub` (#457) * feat: add `handleSynPackage` to `StakeHub` * fix lint issue * fix review comments * fix unit tests * remove `delegateVotingPower` from `StakeMigrationPackage` * fix review comments * update annotations * feat: add comment for governorProtector (#460) * feat: add comment for governorProtector * feat: add comment for assetProtector * chore: revert transaction when delegator in blacklist (#461) * chore: add `tmpValidatorSetUpdated` event (#462) * fix: sync govToken in `_doMigration` (#463) * fix: add `isAutoUndelegate` for auto `DistributeUndelegatedSynPackage` (#464) * fix: add `isAutoUndelegate` for auto `DistributeUndelegatedSynPackage` * add annotation * chore: fix issues from audit report (#465) * chore: fix issues from audit report * fix unit tests * feat: deprecate `transferOut` in `TokenHub` (#466) * feat: deprecate `transferOut` in `TokenHub` * fix review comments * fix: wrong jail time in `_checkValidatorSelfDelegation` (#471) * feat: add `whenNotPaused` to slash functions (#472) * chore: add annotation (#474) * chore: add annotation * chore: add annotation * fix: disable burn in GovToken (#473) * fix: disable burn in GovToken contract * chores: add comments * fix: check approval address is not 0x0 (#476) * feat: emit NotBoundToken event (#479) * feat: emit NotBoundToken event * chore: add comments * chore: update contracts init params to be consistent with mainnet setting (#480) * docs: update tokenhub abi (#482) * fix: add upper limit for some params (#481) * fix: add upper limit for some params * fix lint issue * chore: fix review comments (#483) * chore: fix audit issues (#485) * chore: fix audit issues * fix review comments * fix review comments * fix review comments * chore: refactor codes (#486) * chore: refactor codes * fix review comments * fmt script * chore: fix review comments (#489) * fix: wrong flags passed in generate:dev (#490) * feat: add governor test (#491) * feat: add governor test * fix: rebase interface for stakeHub in latest bc-fusion * fix test cases * rename modifier name --------- Co-authored-by: Roshan <luoshen1997@gmail.com> * test: add unit-test for TokenRecoverPortal (#492) * fix report 8 issue 1 * fix report 8 issue 2 * fix report 9 issue 2 * fix report 9 issue 4 * fix report 9 issue 7 * fix ToB report issue 5 * fix ToB report issue 1 * test: add failed cases to TokenRecoverPortal unit-test (#495) * test: add failed cases to token recover portal ut * chore: emit owner address after TokenRecoverRequested * docs: update token recover portal abi (#498) * fix review comments * fix lint issue * fix: improve the code readablity (#499) * fix: improve the code readablity * fix: change the wrong comment * fix: add check in `_forceMaintainingValidatorsExit` (#500) * fix: add check in `_forceMaintainingValidatorsExit` * add `latestConsensusAddress` * chore: update npm script (#501) * chore: update generate script * change default `SOURCE_CHAIN_ID` in `TokenRecoverPortal` * update testnet asset protector address * update testnet INIT_VOTING_DELAY * fix ut * feat: add more tests for governor (#502) * feat: remove whitelist check while propose, move the check to queue and execute (#503) * fix: add check to make sure there is at least one validator in `_forceMaintainingValidatorsExit` (#504) * fix: add check to make sure there is at least one validator in `_forceMaintainingValidatorsExit` * optimize code * fix: add empty address check and clear state before extracall (#507) * chore: update tokenrecoverportal.abi (#508) * chore: update testnet legacy address bytes (#509) --------- Co-authored-by: Roshan <48975233+Pythonberg1997@users.noreply.github.com> Co-authored-by: Ethan <cosinlinker@gmail.com> Co-authored-by: dylanhuang <j75689@gmail.com> Co-authored-by: Roshan <luoshen1997@gmail.com> Co-authored-by: buddho <galaxystroller@gmail.com>
Clifforddu
reviewed
Aug 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR is to fix some review comments.
Changes
Notable changes: