Skip to content

Commit 7e1624d

Browse files
resolve issues
1 parent 3a19cfd commit 7e1624d

File tree

11 files changed

+103
-44
lines changed

11 files changed

+103
-44
lines changed

l1-contracts/contracts/bridge/asset-tracker/GWAssetTracker.sol

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,6 @@ contract GWAssetTracker is AssetTrackerBase, IGWAssetTracker {
575575
SavedTotalSupply memory tokenSavedTotalSupply = savedChainBalance[_chainId][_migrationNumber][_assetId];
576576
if (!tokenSavedTotalSupply.isSaved) {
577577
// First time accessing this balance for this migration number
578-
// Save the current balance and reset the chainBalance to 0
579578
tokenSavedTotalSupply.amount = chainBalance[_chainId][_assetId];
580579
// Persist the saved balance for this specific migration
581580
savedChainBalance[_chainId][_migrationNumber][_assetId] = SavedTotalSupply({

l1-contracts/contracts/bridge/asset-tracker/IL1AssetTracker.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ interface IL1AssetTracker {
2323

2424
function receiveGatewayToL1MigrationOnL1(FinalizeL1DepositParams calldata _finalizeWithdrawalParams) external;
2525

26-
function migrateTokenBalanceFromNTVV31(bytes32 _assetId) external;
26+
function registerLegacyToken(bytes32 _assetId) external;
2727

2828
function consumeBalanceChange(
2929
uint256 _callerChainId,

l1-contracts/contracts/bridge/asset-tracker/IL2AssetTracker.sol

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ interface IL2AssetTracker {
1111
/// @param amount The amount being migrated
1212
event L1ToGatewayMigrationInitiated(bytes32 indexed assetId, uint256 chainId, uint256 amount);
1313

14-
function setAddresses(uint256 _l1ChainId, bytes32 _baseTokenAssetId) external;
14+
function initL2(
15+
uint256 _l1ChainId,
16+
bytes32 _baseTokenAssetId,
17+
bool _needBasewTokenTotalSupplyBackfill
18+
) external;
1519

1620
function handleInitiateBridgingOnL2(
1721
uint256 _toChainId,

l1-contracts/contracts/bridge/asset-tracker/L1AssetTracker.sol

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,21 @@ import {IChainAssetHandlerBase} from "../../core/chain-asset-handler/IChainAsset
5353
import {IL1MessageRoot} from "../../core/message-root/IL1MessageRoot.sol";
5454

5555
contract L1AssetTracker is AssetTrackerBase, IL1AssetTracker {
56-
/// @dev Per-(chainId, assetId) migration accounting stored on L1.
56+
/// @notice Per-(chainId, assetId) migration accounting stored on L1.
5757
/// @param preV31ChainBalance Chain balance right before the v31 migration.
5858
/// - For non-native tokens it is exactly equal to chainBalance before the *ecosystem* upgraded to v31 (0 for new tokens).
5959
/// - For tokens native to the chain, we imagine that it received 2^256-1 token deposit at the inception point and so
6060
/// all the balances that are not present on the chain are from claimed withdrawals, i.e. for a token that was bridged
6161
/// before v31 it is equal to `2^256-1 - <sum of other chainBalances, including l1>`. For new tokens it is exactly `2^256-1`.
62-
/// @param totalDepositedFromL1 Total amount deposited from L1 to the chain since v31 accounting started.
63-
/// @param totalClaimedOnL1 Total amount claimed on L1 (withdrawals and failed deposits) since v31 accounting started.
62+
/// @param totalDepositedFromL1 Total amount deposited from L1 to the chain since v31 accounting started. Note, that it is not
63+
/// just about any L1->L2 deposit, but only those that debited the chainBalance on L1 directly and it is assumed that every such
64+
/// deposit will be processed while the chain is still settling on L1. It is the responsbility of the chain admin to ensure that.
65+
/// @param totalClaimedOnL1 Total amount claimed on L1 (withdrawals and failed deposits) since v31 accounting started. Note, that it is not just
66+
/// about claim, but claims that affect `chainBalance` of the chain (i.e. the respective failed deposits or withdrawals were submitted
67+
/// while the chain was settling on L1)).
68+
/// @dev It is the responsibility of the *chain* and its admin to ensure that all deposits are processed before the migration to Gateway is complete
69+
/// and vice versa, i.e. all deposits are either fully processed on L1 or fully processed while it settles on ZK Gateway. In case the chain violates
70+
/// this rule, invalid migration amount can be migrated, but it must only affect the chain and its users.
6471
struct InteropL1Info {
6572
uint256 preV31ChainBalance;
6673
uint256 totalDepositedFromL1;
@@ -135,14 +142,13 @@ contract L1AssetTracker is AssetTrackerBase, IL1AssetTracker {
135142
/// @dev Note, that this function performs O(number of chains) calls to NTV. It relies on the fact
136143
/// that the max number of chains is bound by 100, so this function should be always processable.
137144
/// @param _assetId The asset id of the token to migrate the token balance for.
138-
function migrateTokenBalanceFromNTVV31(bytes32 _assetId) public {
145+
function registerLegacyToken(bytes32 _assetId) public {
139146
IL1NativeTokenVault l1NTV = IL1NativeTokenVault(address(NATIVE_TOKEN_VAULT));
140147
uint256 originChainId = NATIVE_TOKEN_VAULT.originChainId(_assetId);
141148
require(originChainId != 0, InvalidChainId());
142149

143150
// This function is only intended to be used for legacy tokens that have not yet been registered.
144151
require(!isTokenRegistered[_assetId], "Max chain balance already assigned");
145-
isTokenRegistered[_assetId] = true;
146152

147153
uint256[] memory allZKChainIds = BRIDGE_HUB.getAllZKChainChainIDs();
148154

@@ -154,7 +160,7 @@ contract L1AssetTracker is AssetTrackerBase, IL1AssetTracker {
154160
// chainBalance inside the L1AT should never be incremented until the token is registered.
155161
require(chainBalance[chainId][_assetId] == 0, "Chain balance already set");
156162

157-
// Origin chain id will be handled later.
163+
// Origin chain id will be handled later in this function.
158164
if (chainId == originChainId) {
159165
continue;
160166
}
@@ -172,8 +178,10 @@ contract L1AssetTracker is AssetTrackerBase, IL1AssetTracker {
172178
if (originChainId != block.chainid) {
173179
address tokenAddress = NATIVE_TOKEN_VAULT.tokenAddress(_assetId);
174180
// Note, that here we have an implicit invariant that the token's total supply
175-
// can never be changed before this migration happens.
176-
// So until a token is registered, all withdrawals must fail.
181+
// can never be changed before this migration happens. So until a token is registered, all withdrawals must fail.
182+
// Note, that if a token is a bridged token native to L2, its representation on L1
183+
// is deployed by NativeTokenVault as `BridgedStandardERC20`, so we can safely assume the returned value
184+
// will be correct.
177185
uint256 migratedBalance = IERC20(tokenAddress).totalSupply();
178186
chainBalance[block.chainid][_assetId] = migratedBalance;
179187
interopInfo[block.chainid][_assetId].preV31ChainBalance = migratedBalance;
@@ -360,7 +368,7 @@ contract L1AssetTracker is AssetTrackerBase, IL1AssetTracker {
360368

361369
// We check the assetId to make sure the chain is not lying about it.
362370
DataEncoding.assetIdCheck(data.tokenOriginChainId, data.assetId, data.originToken);
363-
_requireMigratedFromNTV(data.chainId, data.assetId);
371+
_autoRegisterTokenFromMigration(data.tokenOriginChainId, data.assetId);
364372

365373
uint256 currentSettlementLayer = BRIDGE_HUB.settlementLayer(data.chainId);
366374
require(currentSettlementLayer != block.chainid, NotMigratedChain());
@@ -389,7 +397,6 @@ contract L1AssetTracker is AssetTrackerBase, IL1AssetTracker {
389397
require(fromChainBalance >= amountToKeep, InvalidMigrationAmount(fromChainBalance, amountToKeep));
390398
uint256 amountToMigrate = fromChainBalance - amountToKeep;
391399

392-
_autoRegisterTokenFromMigration(data.tokenOriginChainId, data.assetId);
393400
_migrateFunds({
394401
_fromChainId: data.chainId,
395402
_toChainId: currentSettlementLayer,
@@ -467,7 +474,7 @@ contract L1AssetTracker is AssetTrackerBase, IL1AssetTracker {
467474
/// @notice used to pause deposits on Gateway from L1 for migration back to L1.
468475
function requestPauseDepositsForChainOnGateway(uint256 _chainId) external onlyChain(_chainId) {
469476
uint256 settlementLayer = BRIDGE_HUB.settlementLayer(_chainId);
470-
require(settlementLayer != 0, InvalidSettlementLayer());
477+
require(settlementLayer != block.chainid, InvalidSettlementLayer());
471478
_sendToChain(
472479
settlementLayer,
473480
GW_ASSET_TRACKER_ADDR,

l1-contracts/contracts/bridge/asset-tracker/L2AssetTracker.sol

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ import {
3535
import {AssetTrackerBase} from "./AssetTrackerBase.sol";
3636
import {IL2AssetTracker} from "./IL2AssetTracker.sol";
3737

38+
struct ZKsyncOSBaseTokenV31MigrationStatus {
39+
bool needsBackFill;
40+
uint256 totalSupply;
41+
}
42+
3843
contract L2AssetTracker is AssetTrackerBase, IL2AssetTracker {
3944
struct InteropL2Info {
4045
uint256 totalWithdrawalsToL1;
@@ -60,8 +65,14 @@ contract L2AssetTracker is AssetTrackerBase, IL2AssetTracker {
6065
/// could image there was a big successful deposit at the inception time of 2^256-1 and then the withdrawals behaved the same way as for
6166
/// the bridged L2 tokens.
6267
/// @dev For native tokens, it is expected to be populated atomatically with `isTokenRegistered[block.chainid]`.
68+
/// @dev IMPORTANT: for base token this value may not be correct for zksync os chains until the totalSupply for the base
69+
/// token has been backfilled, so before using this value for the base token, one should check that it was set (`zkSyncOSBaseTokenV31MigrationStatus.needsBackFill = false`).
6370
mapping(bytes32 assetId => SavedTotalSupply snapshot) internal totalPreV31TotalSupply;
6471

72+
/// @dev On zkSync os chains, the `totalSupply()` of the base token is not available by default,
73+
/// so before we ever use it to do any migrations, we need to backfill it.
74+
bool public needBasewTokenTotalSupplyBackfill;
75+
6576
modifier onlyUpgrader() {
6677
if (msg.sender != L2_COMPLEX_UPGRADER_ADDR) {
6778
revert Unauthorized(msg.sender);
@@ -90,13 +101,42 @@ contract L2AssetTracker is AssetTrackerBase, IL2AssetTracker {
90101
_;
91102
}
92103

104+
// FIXME: this function will have to be called by the chain admin after the v31 upgrade to backfill the data.
105+
// It will be fixed in a separate PR with the base token holder PR.
106+
function backFillZKSyncOSBaseTokenV31MigrationData(
107+
uint256 _amount
108+
) external onlyUpgrader {
109+
require(needBasewTokenTotalSupplyBackfill, "Backfill not needed");
110+
111+
// We expect that method to be called after the `totalSupply()` has been already updated
112+
// to the correct one, so we can just register the token.
113+
if (!isTokenRegistered[BASE_TOKEN_ASSET_ID]) {
114+
registerLegacyToken(BASE_TOKEN_ASSET_ID);
115+
return;
116+
}
117+
118+
// We expect that for all registered tokens, the zero `totalPreV31TotalSupply` should be saved.
119+
120+
// The requires below should never be hit, these are just invariant checks.
121+
require(totalPreV31TotalSupply[BASE_TOKEN_ASSET_ID].isSaved, "Total supply should be saved for registered token");
122+
require(totalPreV31TotalSupply[BASE_TOKEN_ASSET_ID].amount == 0, "Total supply should be 0 before backfill");
123+
totalPreV31TotalSupply[BASE_TOKEN_ASSET_ID].amount = _amount;
124+
125+
needBasewTokenTotalSupplyBackfill = false;
126+
}
127+
93128
/// @notice Sets the L1 chain ID and base token asset ID for this L2 chain.
94129
/// @dev This function is called during contract initialization or upgrades.
95130
/// @param _l1ChainId The chain ID of the L1 network.
96131
/// @param _baseTokenAssetId The asset ID of the base token used for gas fees on this chain.
97-
function setAddresses(uint256 _l1ChainId, bytes32 _baseTokenAssetId) external onlyUpgrader {
132+
function initL2(
133+
uint256 _l1ChainId,
134+
bytes32 _baseTokenAssetId,
135+
bool _needBasewTokenTotalSupplyBackfill
136+
) external onlyUpgrader {
98137
L1_CHAIN_ID = _l1ChainId;
99138
BASE_TOKEN_ASSET_ID = _baseTokenAssetId;
139+
needBasewTokenTotalSupplyBackfill = _needBasewTokenTotalSupplyBackfill;
100140
}
101141

102142
function _l1ChainId() internal view returns (uint256) {
@@ -136,11 +176,7 @@ contract L2AssetTracker is AssetTrackerBase, IL2AssetTracker {
136176

137177
/// @notice Stores token total supply snapshot used for pre-v31 migration accounting.
138178
/// @dev Anyone can call this to eagerly initialize the snapshot before the first bridge operation.
139-
function registerLegacyToken(bytes32 _assetId) external override {
140-
_registerLegacyTokenEntry(_assetId);
141-
}
142-
143-
function _registerLegacyTokenEntry(bytes32 _assetId) internal {
179+
function registerLegacyToken(bytes32 _assetId) public override {
144180
if (isTokenRegistered[_assetId]) {
145181
return;
146182
}
@@ -346,6 +382,7 @@ contract L2AssetTracker is AssetTrackerBase, IL2AssetTracker {
346382

347383
address tokenAddress = _tryGetTokenAddress(_assetId);
348384
uint256 readTotalPreV31TotalSupply = _registerLegacyTokenIfNeeded(_assetId, tokenAddress);
385+
require(!needBasewTokenTotalSupplyBackfill, "The base token total supply needs to be backfilled");
349386

350387
uint256 originChainId = L2_NATIVE_TOKEN_VAULT.originChainId(_assetId);
351388
address originalToken = L2_NATIVE_TOKEN_VAULT.originToken(_assetId);

l1-contracts/contracts/bridge/ntv/NativeTokenVaultBase.sol

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -422,20 +422,21 @@ abstract contract NativeTokenVaultBase is
422422
bytes32 _assetId,
423423
address _originalCaller
424424
) internal {
425+
// Note, that in order to track `totalPreV31TotalSupply` correctly in L2AssetTracker,
426+
// we have to call it before the any balance changes will be perforemd.
427+
_handleBridgeToChain(_chainId, _assetId, _depositAmount);
428+
425429
if (_assetId == _baseTokenAssetId()) {
426430
require(_depositAmount == msg.value, ValueMismatch(_depositAmount, msg.value));
427431
if (_isBridgedToken) {
428432
// slither-disable-next-line arbitrary-send-eth
429433
L2_BASE_TOKEN_SYSTEM_CONTRACT.burnMsgValue{value: msg.value}();
430434
}
431-
_handleBridgeToChain(_chainId, _assetId, _depositAmount);
432435
} else {
433436
require(msg.value == 0, NonEmptyMsgValue());
434437
if (_isBridgedToken) {
435438
IBridgedStandardToken(_tokenAddress).bridgeBurn(_originalCaller, _depositAmount);
436-
_handleBridgeToChain(_chainId, _assetId, _depositAmount);
437439
} else {
438-
_handleBridgeToChain(_chainId, _assetId, _depositAmount);
439440
if (!_depositChecked) {
440441
uint256 expectedDepositAmount = _depositFunds(
441442
_originalCaller,

l1-contracts/contracts/l2-upgrades/L2GenesisForceDeploymentsHelper.sol

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,13 @@ library L2GenesisForceDeploymentsHelper {
265265
_isGenesisUpgrade: _isGenesisUpgrade,
266266
_isZKsyncOS: _isZKsyncOS
267267
});
268-
_finalizeDeployments(_ctmDeployer, fixedForceDeploymentsData, additionalForceDeploymentsData);
268+
_finalizeDeployments(
269+
_ctmDeployer,
270+
fixedForceDeploymentsData,
271+
additionalForceDeploymentsData,
272+
_isZKsyncOS,
273+
_isGenesisUpgrade
274+
);
269275
}
270276

271277
function _setupProxyAdmin() private {
@@ -506,7 +512,9 @@ library L2GenesisForceDeploymentsHelper {
506512
function _finalizeDeployments(
507513
address _ctmDeployer,
508514
FixedForceDeploymentsData memory fixedForceDeploymentsData,
509-
ZKChainSpecificForceDeploymentsData memory additionalForceDeploymentsData
515+
ZKChainSpecificForceDeploymentsData memory additionalForceDeploymentsData,
516+
bool _isZKsyncOS,
517+
bool _isGenesisUpgrade
510518
) private {
511519
// It is expected that either through the force deployments above
512520
// or upon initialization, both the L2 deployment of BridgeHub, AssetRouter, and MessageRoot are deployed.
@@ -521,9 +529,12 @@ library L2GenesisForceDeploymentsHelper {
521529
_chainRegistrationSender: fixedForceDeploymentsData.aliasedChainRegistrationSender
522530
});
523531

524-
L2AssetTracker(L2_ASSET_TRACKER_ADDR).setAddresses(
532+
L2AssetTracker(L2_ASSET_TRACKER_ADDR).initL2(
525533
fixedForceDeploymentsData.l1ChainId,
526-
additionalForceDeploymentsData.baseTokenBridgingData.assetId
534+
additionalForceDeploymentsData.baseTokenBridgingData.assetId,
535+
// The only chains that need backfill for the base token's total supply are zksync os
536+
// chains that existing before the v31 upgrade (i.e. isGenesis is false).
537+
_isZKsyncOS && !_isGenesisUpgrade
527538
);
528539

529540
GWAssetTracker(GW_ASSET_TRACKER_ADDR).setAddresses(fixedForceDeploymentsData.l1ChainId);

l1-contracts/deploy-scripts/upgrade/v31/ChainUpgrade_v31.s.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ contract ChainUpgrade_v31 is DefaultChainUpgrade {
3737
uint256 bridgedTokensCount = ntv.bridgedTokensCount();
3838
for (uint256 i = 0; i < bridgedTokensCount; ++i) {
3939
bytes32 assetId = ntv.bridgedTokens(i);
40-
l1AssetTracker.migrateTokenBalanceFromNTVV31(assetId);
40+
l1AssetTracker.registerLegacyToken(assetId);
4141
}
4242
}
4343

l1-contracts/deploy-scripts/upgrade/v31/EcosystemUpgrade_v31.s.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ contract EcosystemUpgrade_v31 is DefaultEcosystemUpgrade {
153153

154154
// Call AssetTracker to migrate the balance
155155
vm.broadcast(getBroadcasterAddress());
156-
assetTracker.migrateTokenBalanceFromNTVV31(assetId);
156+
assetTracker.registerLegacyToken(assetId);
157157

158158
console.log(" Migration successful");
159159
}

0 commit comments

Comments
 (0)