Assessment reportsPublic findings
Back to Zellic site
↗
Assessment reports>Facet Migrations>Scope Extension>Scope extension
GeneralOverview
Findings
High (1)
Low (1)
Informational (1)
DiscussionFactoryRouterPair
Scope ExtensionScope extension
Audit ResultsAssessment Results

Scope extension

Final migration stage

The scope of this engagement was extended to include the review the implementation of the final stage of the Facet v0->v1 migration, which involves the MigrationManager contract.

This L2 system contract is invoked automatically on the first L2 block to complete the migration. Note that this audit extension did not cover the entire migration process -- most of the migration is performed by other out of scope code which creates the initial genesis state.

This review also assumed that the MigrationManager contract storage was correctly initialized to reflect the tasks required to complete the migration, including the set of ERC20 tokens, the mapping of ERC20 token holders, the set of ERC721 contracts, the mapping of ERC721 token IDs, the set of FacetSwap factories and the mapping of factories to deployed FacetSwap pairs.

The final migration stage is needed to emit events that allow offchain indexers to build an up to date view of the balances of the Facet ERC20/ERC721/FacetSwap contracts (and of the existing pairs). The MigrationManager contract invokes some privileged functions on the respective contracts which in turn emit the events.

The engagement fixed the following objectives:

  • ensuring MigrationManager is invoked once, and exactly once, at the appropriate time

  • ensuring the events emitted during the migration provide an accurate view of the token balances and FacetSwap pair data

  • ensuring all privileged functions enforce strict access controls to prevent unauthorized actions

  • ensuring that if the migration fails for any reason the chain cannot progress, preventing state inconsistencies

The audit was performed by reviewing the changes between base commit and commit , and only considering relevant changes to the following files:

  • contracts/src/predeploys/MigrationManager.sol: system contract called automatically by the node on the first block (from the system address 0xDeaDDEaDDeAdDeAdDEAdDEaddeAddEAdDEAd0001) to finish the migration. Causes events to be emitted, allowing token holders and their balances to be indexed.

  • FacetERC20.sol, FacetERC721.sol, MigrationLib.sol in contracts/src/libraries/: invoked by the MigrationManager to emit events

  • contracts/src/predeploys/FacetSwapFactoryVac5.sol: invoked by the MigrationManager to emit events

  • lib/geth_driver.rb and app/models/facet_transaction.rb: part of the Facet node core, these files contain the code that triggers the final migration stage

Discussion of MigrationManager executeMigration function

The executeMigration function is invoked on the first block of Facet L2 to perform the final migration steps. The function can only be invoked from the system address 0xDeaDDEaDDeAdDeAdDEAdDEaddeAddEAdDEAd0001, and only until the migration has not finished.

The migration is performed in batches, emitting 100 events per batch; the executeMigration function is invoked repeatedly until the migration is completed. The first time the function is invoked the total number of events to emit is computed. This number is used to compute the number of batches needed and as a sanity check to ensure all needed events have been emitted.

executeMigration processes the pending tasks in a specific order, by invoking functions that perform migration tasks for FacetSwap pairs, ERC20 tokens, and ERC721 tokens.

The functions are always invoked; they process sets of pending factories/pairs, ERC20 tokens, and ERC721 tokens respectively. Once an element has been processed, it is removed from the set to avoid processing it again. After each action that can emit an event, the functions check whether the batch is complete via the batchFinished function. If the batch is complete, the functions return without doing further processing. The remaining pending elements in the sets will be processed in subsequent invocations of executeMigration.

processFactories

This function performs the migration tasks needed to emit events that give offchain indexers information about FacetSwap pairs.

The function iterates over each FacetSwap pair deployed, doing the following:

  • call migrateERC20 to perform the migration of two ERC20 tokens forming the pair

  • call emitPairCreateEventIfNecessary

  • call migrateERC20 to perform the migration of ERC20 token representing the pair LP tokens

  • call pair.sync() to ensure the pair liquidity reflects the pair liquidity balance

    • NOTE: this causes two events to be emitted, PreSwapReserves and Sync, which are not accounted for (or not entirely, since currentBatchEmittedEvents is only incremented once)

The emitPairCreateEventIfNecessary is called to emit a PairCreated event which allows offchain indexers to record which pairs have been deployed. The function is a no-op if the event is already emitted for a given pair. If the corresponding event has not been emitted yet, the function invokes emitPairCreated on the factory that created the pair. The factory ensures that the caller address is the migration manager contract and emits the PairCreated event.

The migrateERC20 function is described in the following section discussing processERC20Tokens.

processERC20Tokens

This function performs the migration of ERC20 tokens, emitting events that allow offchain indexers to build a view of the balances of all ERC20 holders.

It iterates over the allERC20Tokens set, which contains the addresses of all the ERC20 tokens. Note that ERC20 tokens that are included in a FacetSwap pair have already been processed by processFactories and removed from allERC20Tokens, and are not processed twice.

The migrateERC20 function is called for every ERC20 address in allERC20Tokens. The addresses of the holders of the ERC20 token are read from the erc20TokenToHolders mapping. If the balance of the holder (read from the ERC20 contract) is greater than zero then the emitTransferEvent function is called on the ERC20 contract.

emitTransferEvent checks that the caller is the migration manager and emits a Transfer event. The event to and amount fields correspond to the holder address and their balance. The from field is set to the zero address. Indexers are therefore able to derive the balances of all ERC20 holders but not the source of the balances from the events emitted during the migration.

processERC721Tokens

This function performs the migration of ERC721 tokens, emitting events that allow offchain indexers to build a view of the holders of all ERC721 assets.

It iterates over the address of every deployed ERC721 contract by reading the allERC721Tokens set.

The migration for each individual ERC721 contract is handled by the migrateERC721 function, which iterates over the existing token IDs by reading the set of existing token IDs from the erc721TokenToTokenIds mapping. The owner of the asset is retrieved from the ERC721 contract. If the owner returned by the ERC721 contract is not the zero address, the emitTransferEvent function is invoked on the ERC721 contract. emitTransferEvent checks that the caller is the migration manager contract, and emits a Transfer event. The to and id fields correspond to the owner and the asset ID being processed, while the from field is set to the zero address. By reading these events, indexers are therefore able to derive a mapping associating each existing ERC721 asset to its holder (and vice versa), but not the source of the ERC721 assets.

Note that the function cannot distinguish between nonexisting token IDs and ERC721 tokens that were sent to the zero address. Therefore, no events will be emitted for any ERC721 tokens that were transferred to the zero address.

Small gas inefficiency

Every call to executeMigration terminates with an event emission:

emit BatchComplete(
    currentBatchEmittedEvents,
    remainingEvents,
    calculateTotalEventsToEmit(),
    transactionsRequired()
);

This is inefficient, since the third field is computed by calling calculateTotalEventsToEmit, and the fourth field calls transactionsRequired which also uses calculateTotalEventsToEmit. Since calculateTotalEventsToEmit performs a number of operations proportional to the total number of ERC20, ERC721, and FacetSwapFactory contracts registered in the system, avoiding this repeated call should save a significant amount of gas and might allow a higher number of events per batch.

Mismatch between calculateTotalEventsToEmit and actual number of emitted events

The calculateTotalEventsToEmit function computes the number of events expected to be emitted by the migration process. The total number is the sum of:

  1. the total number of ERC20 token holders; that is the sum of all the lengths of the sets of holder addresses recorded in the erc20TokenToHolders mapping (assuming allERC20Tokens has an entry for each key in the mapping)

    • note that these mappings also record ERC20 tokens that represent LP positions in FacetSwap pairs

  2. the total number of ERC721 assets; that is the total number of individual token IDs for all ERC721 contracts recorded in the erc721TokenToTokenIds mapping (assuming allERC721Tokens has an entry for each key in the mapping)

  3. the total number of FactorySwap pairs times two

The number of events actually emitted is the sum of the events emitted by three functions.

We note that in some cases there may be a mismatch between the number of events actually emitted by the code, and the counters maintained by the code; this does not impact the successful execution of the migration, and we consider this as a naming issue rather than a security issue with a practical impact.

Events emitted by processFactories

For every factory and every pair, this function emits the following events that contribute to the first component of the sum returned by calculateTotalEventsToEmit:

  • one Transfer event for each holder of the first token forming the FacetSwap pair with a greater than zero balance

  • one Transfer event for each holder of the second token forming the FacetSwap pair with a greater than zero balance

  • one Transfer event for each holder of a nonzero balance of the ERC20 token representing pair liquidity

Note 1: that these events are only emitted once for each unique (ERC20, holder) pair; they are not emitted multiple times if the token appears in multiple pairs, nor are they emitted again by processERC20Tokens. This means that if two pairs USDC<->USDT and USDT<->ETH exist, events recording USDT holders are emitted only once by the migration process.

Note 2: the currentBatchEmittedEvents counter is increased even if the holder has a nonzero balance (but the caveat about an ERC20 being processed only once still apply), therefore currentBatchEmittedEvents at the end of the migration currentBatchEmittedEvents will be increased by a total that matches the total number of holders of unique tokens associated with FacetSwap pairs (either as assets or LP tokens).

The function also emits the following events, which contribute to the third component of the total returned by calculateTotalEventsToEmit:

  • one PairCreated event for each pair

  • one PreSwapReserves and one Sync event for each pair, due to a call to pair.sync()

Note 3: the currentBatchEmittedEvents counter is increased by one when PairCreated is emitted, and by one (and not two) when PreSwapReserves and Sync are emitted. Therefore, currentBatchEmittedEvents undercounts the actual number of events.

Events emitted by processERC20Tokens

For every holder of every ERC20 token with a greater than zero balance that was not already processed by processFactories, this function emits one Transfer event. These events contribute to the first component of the sum returned by calculateTotalEventsToEmit.

Note: as with processFactories, the currentBatchEmittedEvents variable is increased even if the holder has a nonzero balance, therefore currentBatchEmittedEvents at the end of the migration currentBatchEmittedEvents will be increased by a total that matches the total number of holders of ERC20 tokens that are not associated with any FacetSwap pair.

The total number of times the currentBatchEmittedEvents counter is increased is therefore equal to the total number of holders of every unique ERC20 asset recorded in erc20TokenToHolders (including holders with zero balances).

Events emitted by processERC721Tokens

This function emits one Transfer event for every token ID of every ERC721 contract, excluding token IDs owned by the zero address. These events contribute to the second component of the sum returned by calculateTotalEventsToEmit.

The currentBatchEmittedEvents variable is increased even if the holder is the zero address, therefore currentBatchEmittedEvents will be increased by the total number of token IDs recorded in the erc721TokenToTokenIds mapping, which might be greater than the actual number of emitted events if any tokens are owned by the zero address.

Conclusions

No exploitable security issues emerged from this review, and the objectives defined for this scope extension were met successfully. The notes above were acknowledged by the Facet team.

Zellic © 2025Back to top ↑