QA Report

[code423n4]

Report

01: PromiseRouter.initialize() doesn't initialize the ReentrancyUpgradable dependency

In your initialize() function you should also call the initialize() functions of all the other contracts you inherit. In this case, the ReentrancyGuardUpgradable one is missing.

Relevant code:

• https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/promise/PromiseRouter.sol#L146

02: Use a two-step process for critical address changes

You already do that in a couple of spots. But, some are missing. For example, the ConnextPriceOracle contract doesn't use it to change the admin address

Relevant code:

• https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L168

03: Multiple open TODOs

Throughout the codebase there are multiple open TODOs.

.//contracts_forge/core/connext/facets/RoutersFacet.t.sol:53:  // TODO: test modifiers? onlyRouterOwner, onlyProposedRouterOwner, etc.
.//contracts_forge/core/connext/facets/BridgeFacet.t.sol:468:        // TODO: For some reason, balance isn't changing. Perhaps the vm.mockCall prevents this?
.//contracts_forge/core/connext/facets/BridgeFacet.t.sol:1310:  // TODO: fails if destination domain does not have an xapp router registered
.//contracts_forge/core/connext/facets/AssetFacet.t.sol:147:  // TODO: test_adminFunctions__onlyOwner ??
.//contracts_forge/core/connext/facets/RelayerFacet.t.sol:44:  // TODO: onlyRelayerFeeRouter
.//contracts_forge/Connext.t.sol:352:    // TODO Correctly calculate the message
.//contracts/core/connext/facets/BridgeFacet.sol:496:      // TODO: do we want to store a mapping of custodied token balances here?
.//contracts/core/connext/facets/BridgeFacet.sol:583:      // TODO: do we need to keep this
.//contracts/core/connext/facets/BridgeFacet.sol:1031:    // TODO: Should we call approve(0) and approve(totalRepayAmount) instead? or with a try catch to not affect gas on all cases?
.//contracts/core/connext/libraries/LibConnextStorage.sol:304:  // BridgeFacet (cont.) TODO: can we move this
.//contracts/core/connext/helpers/Executor.sol:7:// TODO: see note in below file re: npm
.//contracts/nomad-core/contracts/upgrade/UpgradeBeaconController.sol:17:// TODO: upgrade to `ProposedOwnable`
.//contracts/nomad-core/libs/ExcessivelySafeCall.sol:7:// TODO: Update to npm version when these changes are reflected

[jakekidd]

01: invalid - this is handled in BaseConnextFacet.init method