feat: OPCM v2 #18079

smartcontracts · 2025-10-29T18:26:36Z

Description
This PR introduces OPCMv2. Refer to the design document and the product requirements document for more information.

codecov · 2025-10-30T20:49:17Z

Codecov Report

❌ Patch coverage is 90.14778% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.47%. Comparing base (5b96584) to head (a8fe8d1).
⚠️ Report is 13 commits behind head on develop.

Files with missing lines	Patch %	Lines
...racts-bedrock/src/L1/opcm/OPContractsManagerV2.sol	89.94%	19 Missing ⚠️
...edrock/src/L1/opcm/OPContractsManagerContainer.sol	92.85%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #18079      +/-   ##
===========================================
- Coverage    76.58%   76.47%   -0.12%     
===========================================
  Files          179      181       +2     
  Lines        10732    10935     +203     
===========================================
+ Hits          8219     8362     +143     
- Misses        2367     2429      +62     
+ Partials       146      144       -2

Flag	Coverage Δ
cannon-go-tests-64	`66.58% <ø> (-0.82%)`	⬇️
contracts-bedrock-tests	`82.24% <90.14%> (+0.13%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...ages/contracts-bedrock/src/libraries/Constants.sol	`100.00% <ø> (ø)`
...es/contracts-bedrock/src/libraries/DevFeatures.sol	`100.00% <ø> (ø)`
...edrock/src/L1/opcm/OPContractsManagerContainer.sol	`92.85% <92.85%> (ø)`
...racts-bedrock/src/L1/opcm/OPContractsManagerV2.sol	`89.94% <89.94%> (ø)`

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

packages/contracts-bedrock/src/L1/OPContractsManager.sol

mds1 · 2025-11-04T03:57:19Z

packages/contracts-bedrock/src/L1/OPContractsManager.sol

+            ISystemConfig.initialize,
+            (
+                _cfg.systemConfigOwner,
+                _cfg.basefeeScalar,
+                _cfg.blobBasefeeScalar,
+                bytes32(uint256(uint160(_cfg.batcher))),
+                _cfg.gasLimit,
+                _cfg.unsafeBlockSigner,
+                _cfg.resourceConfig,
+                chainIdToBatchInboxAddress(_cfg.l2ChainId),
+                addrs,
+                _cfg.l2ChainId,
+                _cfg.superchainConfig
+            )


Alternatively, let's refactor this initializer to use some structs, then we won't get the stack too deep problem here and it will be easier to extend this in the future

Might be a good future improvement, I didn't want to modify too much in the contracts for now

mds1 · 2025-11-04T04:01:19Z

packages/contracts-bedrock/src/L1/OPContractsManager.sol

+        // Upgrade to StorageSetter.
+        _proxyAdmin.upgrade(payable(_target), address(implementations().storageSetterImpl));
+
+        // Reset the initialized slot by zeroing the single byte at `_offset` (from the right).
+        bytes32 current = StorageSetter(_target).getBytes32(_slot);
+        uint256 mask = ~(uint256(0xff) << (uint256(_offset) * 8));
+        StorageSetter(_target).setBytes32(_slot, bytes32(uint256(current) & mask));
+
+        // Upgrade to the implementation and call the initializer.
+        _proxyAdmin.upgradeAndCall(payable(address(_target)), _implementation, _data);


I wish there were initializer getter methods that Initializable has so we can assert onchain that we properly reset the initializer and did not change anything else, but seems those don't exist

Wonder if it's overkill to fork initializable and add these getters? Would be a consistent philosophy to #18079 (comment) around doing things out of an abundance of caution

packages/contracts-bedrock/interfaces/L1/IOPContractsManager.sol

maurelian

first round of comments and questions

packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol

packages/contracts-bedrock/interfaces/L1/opcm/IOPContractsManagerContainer.sol

packages/contracts-bedrock/src/L1/OPContractsManager.sol

maurelian · 2025-11-18T21:25:17Z

packages/contracts-bedrock/src/L1/OPContractsManager.sol

+        cfg.blobBasefeeScalar = _input.blobBasefeeScalar;
+        cfg.gasLimit = _input.gasLimit;
+        cfg.l2ChainId = _input.l2ChainId;
+        cfg.resourceConfig = Constants.DEFAULT_RESOURCE_CONFIG();


hmmm, why is resourceConfig becoming a configurable input? Is that wanted or needed?

This one is mostly just because it's hard to figure out how to pass this around if you don't have it as a field on FullConfig. It needs to make its way to the _execute function somehow. resourceConfig is already configurable by chain operators if they want it to be, easy enough to just set it to the recommended default value manually.

maurelian · 2025-11-18T21:29:40Z

packages/contracts-bedrock/src/L1/OPContractsManager.sol

+            )
+        });
+        cfg.disputeGameConfigs[1] = IOPContractsManagerV2.DisputeGameConfig({
+            enabled: true,


This is the only one enabled. Is there any benefit to including disable game types in the array or could we cut it down to a single game config?

Yes - including disabled game types in the array forces users to explicitly consider each game type. If you don't force the user to include an input for each game type then they can easily fail to provide an input and potentially misconfigure a game. Better to be very explicit.

packages/contracts-bedrock/src/L1/OPContractsManager.sol

maurelian · 2025-11-18T21:34:32Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerContainer.sol

+    ///         have any code in production environments but can be made to have code in tests.
+    /// @return True if the contract is running in a testing environment, false otherwise.
+    function _isTestingEnvironment() internal view returns (bool) {
+        return address(0xbeefcafe).code.length > 0;


nit: this should be in Constants.sol

packages/contracts-bedrock/snapshots/storageLayout/OPContractsManagerV2.json

packages/contracts-bedrock/scripts/deploy/Deploy.s.sol

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

maurelian · 2025-11-18T21:51:45Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+        address ret;
+        if (LibString.eq(_contractName, "L1StandardBridge")) {
+            // L1StandardBridge is a special case ChugSplashProxy (legacy).
+            ret = Blueprint.deployFrom(
+                blueprints().l1ChugSplashProxy,
+                _computeSalt(_args.l2ChainId, _args.saltMixer, "L1StandardBridge"),
+                abi.encode(_args.proxyAdmin)
+            );
+
+            // ChugSplashProxy requires setting the proxy type on the ProxyAdmin.
+            _args.proxyAdmin.setProxyType(ret, IProxyAdmin.ProxyType.CHUGSPLASH);
+        } else if (LibString.eq(_contractName, "L1CrossDomainMessenger")) {
+            // L1CrossDomainMessenger is a special case ResolvedDelegateProxy (legacy).
+            string memory l1XdmName = "OVM_L1CrossDomainMessenger";
+            ret = Blueprint.deployFrom(
+                blueprints().resolvedDelegateProxy,
+                _computeSalt(_args.l2ChainId, _args.saltMixer, "L1CrossDomainMessenger"),
+                abi.encode(_args.addressManager, l1XdmName)
+            );


Is it better to have this function be really big and detect which contracts get the nonstandard proxies, or should we break them out?

This felt marginally cleaner to me than having three different functions. Do you have a specific alternative in mind?

What about just using an early return pattern instead of if/else? I think that would improve readability somewhat.

I acknowledge this is a bikeshed, if you feel strongly I'm OK with it.

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

packages/contracts-bedrock/test/L1/OPContractsManager.t.sol

maurelian · 2025-11-20T21:08:55Z

packages/contracts-bedrock/test/L1/OPContractsManager.t.sol

        }

+        // All V1 upgrade tests can safely be skipped for V2.
+        skipIfDevFeatureEnabled(DevFeatures.OPCM_V2);


Does this still make sense to use? Maybe it saves some test running time, but the OPCMv1 works the same either way now right?

I'm purposefully making it so the OPCMv1 doesn't work either way by not labeling the OPCMv1 address when OPCMv2 is enabled. This breaks all the tests that rely on OPCMv1 which makes it a lot easier to figure out what coverage we need iteratively (can delete these skips, see what fails, those are the tests we need to copy over to V2). I'll remove all these skips once we're completely happy with the V2 coverage.

packages/contracts-bedrock/test/L1/OPContractsManager.t.sol

packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol

maurelian · 2025-11-20T22:07:09Z

packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol

comment only: fork tests and feature flags look to be quite a hassle in this file.

maurelian · 2025-11-20T22:09:22Z

packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol

+                if (isDevFeatureEnabled(DevFeatures.CANNON_KONA)) {
+                    addGameType(GameTypes.CANNON_KONA, cannonKonaPrestate);
+                }
+            } else {


This block would have made more sense to me on first reading if the comment in FeatureFlags.sol was reflected here too:

OPCMv2 also automatically implies DEPLOY_V2_DISPUTE_GAMES and CANNON_KONA.

0xiamflux · 2025-11-20T22:14:05Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+        _assertValidUpgradeInstructions(_inp.extraInstructions);
+
+        // Load the chain contracts.
+        ChainContracts memory cts =


nit: revert with error in case _inp.systemConfig is address(0).

maurelian · 2025-11-21T02:33:56Z

packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol

+                assertEq("PDDG-DWETH-20,PLDG-DWETH-20,CKDG-DWETH-20", _validate(true));
            } else {
-                assertEq("PLDG-DWETH-20", _validate(true));
+                assertEq("PDDG-DWETH-20,PLDG-DWETH-20", _validate(true));


OPCMv2 also automatically implies DEPLOY_V2_DISPUTE_GAMES and CANNON_KONA.

I think this block is unreachable?

Likewise for a bunch of other occurences in this file.

maurelian · 2025-11-21T02:47:16Z

packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol

-        if (isForkTest()) {
-            assertEq("PDDG-DWETH-30", _validate(true));
-        } else {
+        if (isDevFeatureEnabled(DevFeatures.OPCM_V2)) {


For most occurences in this file I don't see why we need to branch on this flag, as there are no changes to the StandardValidator. Both OPCMs should be able to get the system to the same valid state.

The only place that I think we really need isDevFeatureEnabled(DevFeatures.OPCM_V2) is in setUp() where you decide whether to add the enabled games using either opcm.addGameType() or opcmv2.upgrade().

--

OK, having written this, I realize that the difference is that V2 sets the same WETH for all games, whereas V1 keeps them separate. Would it be easier to just update V1 to act like V2 to save us from all this branching? I suppose that since it's already written we could maybe just live with it for now and look forward to deleting the flag.

I'd rather not update V1 to act like V2. Updating V1 means either committing to immediately shipping this change in V1, or it means adding yet another feature flag in the V1 which we'd probably not actually use because the V1 would be replaced by the V2 by the time it's used.

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

maurelian · 2025-11-21T15:24:03Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+                )
+            );
+        } else {
+            // Special case handling, don't bother with the standard flow.


why is this a "special case" isn't it just what happens whenever this is called by upgrade()?

"Special case" here means that we're not using the _loadOrDeployProxy pattern. I should clarify the comment.

I see. The placement of the comment seems to suggest that it applies to the full block. I think it can be handled just by expanding the text a bit more.

maurelian · 2025-11-21T16:48:03Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+        ExtraInstruction[] memory instructions = new ExtraInstruction[](1);
+        instructions[0] = ExtraInstruction({ key: "PermittedProxyDeployment", data: bytes("ALL") });


I think that this has become slightly confusing now that fd308b4 has added an ExtraInstruction field to FullConfig, ie. there is :

_cfg.extraInstructions provided as an arg to deploy() which is passed into _execute(), and

instructions created here and passed into _loadChainContracts()

WDYT if _loadChainContracts() accepted a boolean instead, and then use that boolean to create the PermittedProxyDeployment: ALL instruction inline?

Ok will handle it this way. Separately, I think we probably need to add an _assertValidDeployInstructions function.

maurelian · 2025-11-21T18:44:47Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+            return payable(abi.decode(result, (address)));
+        } else if (!loadCanFail) {
+            // Load not permitted to fail but did, revert.
+            revert OPContractsManagerV2_ProxyMustLoad();


Nit: might be nicer for debugging:

Suggested change

revert OPContractsManagerV2_ProxyMustLoad();

revert OPContractsManagerV2_ProxyMustLoad(_contractName);

smartcontracts · 2025-11-21T19:18:26Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+            _loadChainContracts(ISystemConfig(address(0)), _cfg.l2ChainId, _cfg.saltMixer, instructions);
+
+        // Execute the deployment.
+        return _execute(_cfg, cts, true);


Maybe rename to apply

smartcontracts · 2025-11-21T19:57:04Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+        // Dispute game configuration.
+        DisputeGameConfig[] disputeGameConfigs;
+        // Extra deployment instructions.
+        ExtraInstruction[] extraInstructions;


Let's get rid of this

0xniha · 2025-11-21T20:06:50Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+        IProxyAdmin proxyAdmin;
+        IAddressManager addressManager;
+        ISystemConfig systemConfig;
+        if (isInitialDeployment) {


This config could have it's own separate function:

if(isInitialDeployment){ (proxyAdmin, addressManager, systemConfig, ethLockbox) = _setUpInitialDeployment(...); } else { (proxyAdmin, addressManager, systemConfig, ethLockbox) = _setUpUpgrade(...); }

maurelian · 2025-11-21T20:26:51Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+
+    /// @notice Validates the deployment/upgrade config.
+    /// @param _cfg The full config.
+    function _assertValidFullConfig(FullConfig memory _cfg) internal pure {


Instead of validating the input, we should consider updating _execute() to call standardValidator.validateWithOverrides() at the end. The overrides should be taken from the FullConfig.

We would need to add a new string expectedErrors arg, which will allow us to define which errors are allowed. We had discussed including a boolean like useStandardValidator, but that is not necessary, we should always call it and ensure that its response matches expectedErrors.

0xiamflux · 2025-11-24T00:21:28Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+    {
+        bool loadCanFail = _hasInstruction(
+            _instructions, ExtraInstruction({ key: "PermittedProxyDeployment", data: bytes(_contractName) })
+        ) || _hasInstruction(_instructions, ExtraInstruction({ key: "PermittedProxyDeployment", data: bytes("ALL") }));


I think it would be a good idea to remove this magic value ALL and use a constant instead, wdyt?

0xiamflux · 2025-11-24T00:29:28Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+        // We've failed to load, but we allowed that failure.
+        // Deploy the right proxy depending on the contract name.
+        address ret;
+        if (LibString.eq(_contractName, "L1StandardBridge")) {


We might prefer to use a constant instead of hardcoded contract names.

0xiamflux · 2025-11-24T00:29:45Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+
+            // ChugSplashProxy requires setting the proxy type on the ProxyAdmin.
+            _args.proxyAdmin.setProxyType(ret, IProxyAdmin.ProxyType.CHUGSPLASH);
+        } else if (LibString.eq(_contractName, "L1CrossDomainMessenger")) {


0xiamflux · 2025-11-24T00:39:25Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+            _proxyAdmin.getProxyImplementation(payable(_target)) != address(0)
+                && SemverComp.gt(ISemver(_target).version(), ISemver(_implementation).version())
+        ) {
+            revert OPContractsManagerV2_DowngradeNotAllowed();


I think it would be useful to add the contract address that is violating the downgrade check to the error.

revert OPContractsManagerV2_DowngradeNotAllowed(_target);

0xiamflux · 2025-11-24T00:45:55Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+    ///         have any code in production environments but can be made to have code in tests.
+    /// @return True if the contract is running in a testing environment, false otherwise.
+    function _isTestingEnvironment() internal view returns (bool) {
+        return address(0xbeefcafe).code.length > 0;


We could use Constants.TESTING_ENVIRONMENT_ADDRESS here.

0xiamflux · 2025-11-24T00:47:36Z

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

+    /// @notice Helper function to load data from a source contract as bytes.
+    /// @param _source The source contract to load the data from.
+    /// @param _selector The selector of the function to call on the source contract.
+    /// @param _instructions The extra upgrade instructions for the data load.


Missing _name param in natspec.

smartcontracts force-pushed the sc/opcm-v2-v2 branch from d515a99 to 0bdef54 Compare October 29, 2025 18:30

smartcontracts force-pushed the sc/opcm-v2-v2 branch 4 times, most recently from 6fb6a47 to 73eb1b6 Compare November 3, 2025 16:54

mds1 reviewed Nov 4, 2025

View reviewed changes

packages/contracts-bedrock/src/L1/OPContractsManager.sol Outdated Show resolved Hide resolved

mds1 reviewed Nov 4, 2025

View reviewed changes

packages/contracts-bedrock/src/L1/OPContractsManager.sol Outdated Show resolved Hide resolved

mds1 reviewed Nov 4, 2025

View reviewed changes

packages/contracts-bedrock/src/L1/OPContractsManager.sol Outdated Show resolved Hide resolved

mds1 reviewed Nov 4, 2025

View reviewed changes

packages/contracts-bedrock/src/L1/OPContractsManager.sol Outdated Show resolved Hide resolved

mds1 reviewed Nov 4, 2025

View reviewed changes

smartcontracts force-pushed the sc/opcm-v2-v2 branch from 83ace94 to 71c612a Compare November 4, 2025 17:45

smartcontracts force-pushed the sc/opcm-v2-v2 branch 7 times, most recently from 5c89c68 to 049f5bb Compare November 18, 2025 17:30

smartcontracts marked this pull request as ready for review November 18, 2025 19:14

smartcontracts requested review from a team as code owners November 18, 2025 19:14

smartcontracts requested review from mslipper and teddyknox November 18, 2025 19:14

smartcontracts force-pushed the sc/opcm-v2-v2 branch from 4143f86 to 7492dac Compare November 18, 2025 19:15

maurelian reviewed Nov 18, 2025

View reviewed changes

packages/contracts-bedrock/interfaces/L1/IOPContractsManager.sol Show resolved Hide resolved

maurelian reviewed Nov 18, 2025

View reviewed changes

smartcontracts added 2 commits November 19, 2025 10:31

fix: further clean diff

b54a1a3

fix: reduce diff

64f664f

smartcontracts force-pushed the sc/opcm-v2-v2 branch from 7c0f993 to 64f664f Compare November 19, 2025 15:31

smartcontracts added 2 commits November 19, 2025 10:33

fix: undo artifacts diffs

c9ae24d

fix: broken go structs

fade610

maurelian reviewed Nov 19, 2025

View reviewed changes

packages/contracts-bedrock/test/L1/OPContractsManager.t.sol Outdated Show resolved Hide resolved

smartcontracts added 5 commits November 19, 2025 12:14

fix: opcm output struct ordering

9a056e2

fix: better tests

8863da3

fix: justfile path

73f3751

fix: lcov issue

715eeb6

fix: pr review comments

fd308b4

maurelian reviewed Nov 20, 2025

View reviewed changes

packages/contracts-bedrock/test/L1/OPContractsManager.t.sol Show resolved Hide resolved

maurelian reviewed Nov 20, 2025

View reviewed changes

packages/contracts-bedrock/test/L1/OPContractsManagerStandardValidator.t.sol Show resolved Hide resolved

maurelian reviewed Nov 20, 2025

View reviewed changes

0xiamflux reviewed Nov 20, 2025

View reviewed changes

maurelian reviewed Nov 21, 2025

View reviewed changes

packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol Show resolved Hide resolved

maurelian reviewed Nov 21, 2025

View reviewed changes

smartcontracts force-pushed the sc/opcm-v2-v2 branch from a8fe8d1 to fd308b4 Compare November 21, 2025 18:50

smartcontracts commented Nov 21, 2025

View reviewed changes

0xniha reviewed Nov 21, 2025

View reviewed changes

maurelian reviewed Nov 21, 2025

View reviewed changes

maurelian mentioned this pull request Nov 21, 2025

Use vm.prank(address,bool) for delegateCall, remove DelegateCaller contract #18331

Open

0xiamflux reviewed Nov 24, 2025

View reviewed changes

		ExtraInstruction[] memory instructions = new ExtraInstruction[](1);
		instructions[0] = ExtraInstruction({ key: "PermittedProxyDeployment", data: bytes("ALL") });

	revert OPContractsManagerV2_ProxyMustLoad();
	revert OPContractsManagerV2_ProxyMustLoad(_contractName);

feat: OPCM v2 #18079

Are you sure you want to change the base?

feat: OPCM v2 #18079

Conversation

smartcontracts commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maurelian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maurelian Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

smartcontracts commented Oct 29, 2025 •

edited

Loading

codecov bot commented Oct 30, 2025 •

edited

Loading

maurelian Nov 20, 2025 •

edited

Loading