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

fix: EVM-708 bridge fixes #703

Merged
merged 19 commits into from
Aug 16, 2024
Merged

fix: EVM-708 bridge fixes #703

Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
820994b
bridge fixes
kelemeno Aug 15, 2024
1bec619
typo
kelemeno Aug 15, 2024
36aded3
some fixes
kelemeno Aug 15, 2024
870d6e7
build fix
kelemeno Aug 15, 2024
cb91249
remove admin facet
kelemeno Aug 15, 2024
e81b117
fixes
kelemeno Aug 15, 2024
a577980
fix
kelemeno Aug 15, 2024
015f05c
slither issue
kelemeno Aug 15, 2024
d4d547d
solhint
kelemeno Aug 15, 2024
98a053a
more fixes
kelemeno Aug 15, 2024
7dfbefb
slither
kelemeno Aug 15, 2024
77aca06
slither
kelemeno Aug 15, 2024
93217f0
where to put the slither comment
kelemeno Aug 15, 2024
9b1109f
slither
kelemeno Aug 15, 2024
3066c8b
more fixes
kelemeno Aug 16, 2024
485df95
Merge branch 'kl/sync-layer-reorg' of ssh://github.com/matter-labs/er…
kelemeno Aug 16, 2024
915017a
lint
kelemeno Aug 16, 2024
f7891da
fix
kelemeno Aug 16, 2024
a87ab57
old comment
kelemeno Aug 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions l1-contracts/contracts/bridgehub/Bridgehub.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,19 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus
require(token != address(0), "BH: token not set");
baseTokenAssetId[_chainId] = DataEncoding.encodeNTVAssetId(block.chainid, token);
Copy link
Collaborator

@StanislavBreadless StanislavBreadless Aug 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

should be checked that the chain does not yet have the base token asset id set.

Probably reverting in case the token is set is too much (since the method is public and be frontrun), but at least not overwriting the base token should be done in case some chains have custom asset handler for base token

}

/// @notice Used to set the legacy chain address for the upgrade.
/// @notice This has to be used after the BH but before the STM is upgraded.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the comment is not correct, it has to be used after STM is upgraded, since getHyperchainLegacy is available only on the new one

/// @param _chainId The chainId of the legacy chain we are migrating.
function setLegacyChainAddress(uint256 _chainId) external {
address stm = stateTransitionManager[_chainId];
require(stm != address(0), "BH: chain not legacy");
require(!hyperchainMap.contains(_chainId), "BH: chain already migrated");
/// Note we have to do this before STM is upgraded.
address chainAddress = IStateTransitionManager(stm).getHyperchainLegacy(_chainId);
require(chainAddress != address(0), "BH: chain not legacy 2");
_registerNewHyperchain(_chainId, chainAddress);
}

//// Registry

Expand Down
Loading
You are viewing a condensed version of this merge commit. You can view the full changes here.