Skip to content

Commit 1edc4ed

Browse files
authored
[ETHEREUM-CONTRACTS] fix simple forwarder (alternative solution) (#2045)
* deploy SimpleForwarder the same way as ERC2771Forwarder * chore: fix some typos in comment (#2041)
1 parent d199860 commit 1edc4ed

File tree

18 files changed

+106
-83
lines changed

18 files changed

+106
-83
lines changed

packages/automation-contracts/autowrap/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"version": "0.3.0",
55
"devDependencies": {
66
"@openzeppelin/contracts": "^4.9.6",
7-
"@superfluid-finance/ethereum-contracts": "^1.12.0",
7+
"@superfluid-finance/ethereum-contracts": "^1.12.1",
88
"@superfluid-finance/metadata": "^1.5.2"
99
},
1010
"license": "MIT",

packages/automation-contracts/scheduler/contracts/interface/IVestingScheduler.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ interface IVestingScheduler {
5959
* @param superToken SuperToken to be vested
6060
* @param receiver Vesting receiver
6161
* @param startDate Timestamp when the vesting should start
62-
* @param cliffDate Timestamp of cliff exectution - if 0, startDate acts as cliff
62+
* @param cliffDate Timestamp of cliff execution - if 0, startDate acts as cliff
6363
* @param flowRate The flowRate for the stream
6464
* @param cliffAmount The amount to be transferred at the cliff
6565
* @param endDate The timestamp when the stream should stop.

packages/automation-contracts/scheduler/contracts/interface/IVestingSchedulerV2.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ interface IVestingSchedulerV2 {
4545
* @param receiver Vesting receiver
4646
* @param startDate Timestamp when the vesting should start
4747
* @param claimValidityDate Date before which the claimable schedule must be claimed
48-
* @param cliffDate Timestamp of cliff exectution - if 0, startDate acts as cliff
48+
* @param cliffDate Timestamp of cliff execution - if 0, startDate acts as cliff
4949
* @param flowRate The flowRate for the stream
5050
* @param cliffAmount The amount to be transferred at the cliff
5151
* @param endDate The timestamp when the stream should stop.
@@ -98,7 +98,7 @@ interface IVestingSchedulerV2 {
9898
* @param superToken SuperToken to be vested
9999
* @param receiver Vesting receiver
100100
* @param startDate Timestamp when the vesting should start
101-
* @param cliffDate Timestamp of cliff exectution - if 0, startDate acts as cliff
101+
* @param cliffDate Timestamp of cliff execution - if 0, startDate acts as cliff
102102
* @param flowRate The flowRate for the stream
103103
* @param cliffAmount The amount to be transferred at the cliff
104104
* @param endDate The timestamp when the stream should stop.

packages/automation-contracts/scheduler/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"version": "1.3.0",
55
"devDependencies": {
66
"@openzeppelin/contracts": "^4.9.6",
7-
"@superfluid-finance/ethereum-contracts": "^1.12.0",
7+
"@superfluid-finance/ethereum-contracts": "^1.12.1",
88
"@superfluid-finance/metadata": "^1.5.2"
99
},
1010
"license": "MIT",

packages/ethereum-contracts/CHANGELOG.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@ All notable changes to the ethereum-contracts will be documented in this file.
33

44
This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
55

6-
## UNRELEASED
6+
## [v1.12.1]
77

88
### Added
99
- Superfluid Pools now implement `IERC20Metadata`, thus going forward have a name, symbol and decimals
1010
- `ISuperfluidPool.createPoolWithCustomERC20Metadata` for creating pools with custom ERC20 metadata
1111

12+
### Changed
13+
- Fixed deployment of SimpleForwarder (solved an issue which caused batch operation `OPERATION_TYPE_SIMPLE_FORWARD_CALL` to always revert)
14+
1215
## [v1.12.0]
1316

1417
### Added

packages/ethereum-contracts/contracts/mocks/SuperfluidMock.t.sol

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { CallUtils } from "../libs/CallUtils.sol";
1212
contract SuperfluidUpgradabilityTester is Superfluid {
1313

1414
// 3_000_000 is the min callback gas limit used in a prod deployment
15-
constructor() Superfluid(false, false, 3_000_000, address(0))
15+
constructor() Superfluid(false, false, 3_000_000, address(0), address(0))
1616
// solhint-disable-next-line no-empty-blocks
1717
{
1818
}
@@ -134,9 +134,12 @@ contract SuperfluidMock is Superfluid {
134134
bool nonUpgradable,
135135
bool appWhiteListingEnabled,
136136
uint64 callbackGasLimit,
137-
address erc2771Forwarder
137+
address simpleForwarderAddress,
138+
address erc2771ForwarderAddress
138139
)
139-
Superfluid(nonUpgradable, appWhiteListingEnabled, callbackGasLimit, erc2771Forwarder)
140+
Superfluid(
141+
nonUpgradable, appWhiteListingEnabled, callbackGasLimit, simpleForwarderAddress, erc2771ForwarderAddress
142+
)
140143
// solhint-disable-next-line no-empty-blocks
141144
{
142145
}

packages/ethereum-contracts/contracts/superfluid/Superfluid.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ contract Superfluid is
5656
uint64 immutable public CALLBACK_GAS_LIMIT;
5757

5858
// simple forwarder contract used to relay arbitrary calls for batch operations
59-
// Note: address will change with every contract upgrade
6059
SimpleForwarder immutable public SIMPLE_FORWARDER;
6160
ERC2771Forwarder immutable internal _ERC2771_FORWARDER;
6261

@@ -105,13 +104,14 @@ contract Superfluid is
105104
bool nonUpgradable,
106105
bool appWhiteListingEnabled,
107106
uint64 callbackGasLimit,
107+
address simpleForwarderAddress,
108108
address erc2771ForwarderAddress
109109
) {
110110
NON_UPGRADABLE_DEPLOYMENT = nonUpgradable;
111111
APP_WHITE_LISTING_ENABLED = appWhiteListingEnabled;
112112
CALLBACK_GAS_LIMIT = callbackGasLimit;
113+
SIMPLE_FORWARDER = SimpleForwarder(simpleForwarderAddress);
113114
_ERC2771_FORWARDER = ERC2771Forwarder(erc2771ForwarderAddress);
114-
SIMPLE_FORWARDER = new SimpleForwarder();
115115
}
116116

117117
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

packages/ethereum-contracts/contracts/utils/SuperfluidFrameworkDeploymentSteps.t.sol

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import { UUPSProxy } from "../upgradability/UUPSProxy.sol";
2929
import { BatchLiquidator } from "./BatchLiquidator.sol";
3030
import { TOGA } from "./TOGA.sol";
3131
import { IResolver } from "../interfaces/utils/IResolver.sol";
32+
import { SimpleForwarder } from "../utils/SimpleForwarder.sol";
3233
import { ERC2771Forwarder } from "../utils/ERC2771Forwarder.sol";
3334
import { MacroForwarder } from "../utils/MacroForwarder.sol";
3435

@@ -140,10 +141,14 @@ contract SuperfluidFrameworkDeploymentSteps {
140141
testGovernance = SuperfluidGovDeployerLibrary.deployTestGovernance();
141142
SuperfluidGovDeployerLibrary.transferOwnership(testGovernance, address(this));
142143
} else if (step == 1) { // CORE CONTRACT: Superfluid (Host)
143-
ERC2771Forwarder erc2771Forwarder = SuperfluidERC2771ForwarderDeployerLibrary.deploy();
144+
SimpleForwarder simpleForwarder = new SimpleForwarder();
145+
ERC2771Forwarder erc2771Forwarder = new ERC2771Forwarder();
144146
// Deploy Host and initialize the test governance.
145147
// 3_000_000 is the min callback gas limit used in a prod deployment
146-
host = SuperfluidHostDeployerLibrary.deploy(true, false, 3_000_000, address(erc2771Forwarder));
148+
host = SuperfluidHostDeployerLibrary.deploy(
149+
true, false, 3_000_000, address(simpleForwarder), address(erc2771Forwarder)
150+
);
151+
simpleForwarder.transferOwnership(address(host));
147152
erc2771Forwarder.transferOwnership(address(host));
148153

149154
host.initialize(testGovernance);
@@ -308,23 +313,19 @@ library SuperfluidGovDeployerLibrary {
308313
}
309314
}
310315

311-
library SuperfluidERC2771ForwarderDeployerLibrary {
312-
// After deploying, you may want to transfer ownership to the host
313-
function deploy() external returns (ERC2771Forwarder) {
314-
return new ERC2771Forwarder();
315-
}
316-
}
317-
318316
library SuperfluidHostDeployerLibrary {
319317
function deploy(
320318
bool _nonUpgradable,
321319
bool _appWhiteListingEnabled,
322320
uint64 callbackGasLimit,
321+
address simpleForwarderAddress,
323322
address erc2771ForwarderAddress
324323
)
325324
external returns (Superfluid)
326325
{
327-
return new Superfluid(_nonUpgradable, _appWhiteListingEnabled, callbackGasLimit, erc2771ForwarderAddress);
326+
return new Superfluid(
327+
_nonUpgradable, _appWhiteListingEnabled, callbackGasLimit, simpleForwarderAddress, erc2771ForwarderAddress
328+
);
328329
}
329330
}
330331

packages/ethereum-contracts/dev-scripts/deploy-test-framework.js

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ const SuperTokenFactoryDeployerLibraryArtifact = require("@superfluid-finance/et
1818
const SuperfluidFrameworkDeployerArtifact = require("@superfluid-finance/ethereum-contracts/build/hardhat/contracts/utils/SuperfluidFrameworkDeployer.t.sol/SuperfluidFrameworkDeployer.json");
1919
const SlotsBitmapLibraryArtifact = require("@superfluid-finance/ethereum-contracts/build/hardhat/contracts/libs/SlotsBitmapLibrary.sol/SlotsBitmapLibrary.json");
2020
const TokenDeployerLibraryArtifact = require("@superfluid-finance/ethereum-contracts/build/hardhat/contracts/utils/SuperfluidFrameworkDeploymentSteps.t.sol/TokenDeployerLibrary.json");
21-
const SuperfluidERC2771ForwarderDeployerLibraryArtifact = require("@superfluid-finance/ethereum-contracts/build/hardhat/contracts/utils/SuperfluidFrameworkDeploymentSteps.t.sol/SuperfluidERC2771ForwarderDeployerLibrary.json");
2221

2322
const ERC1820Registry = require("../dev-scripts/artifacts/ERC1820Registry.json");
2423

@@ -225,12 +224,6 @@ const _deployTestFramework = async (provider, signer) => {
225224
TokenDeployerLibraryArtifact,
226225
signer
227226
);
228-
const SuperfluidERC2771ForwarderDeployerLibrary =
229-
await _getFactoryAndReturnDeployedContract(
230-
"SuperfluidERC2771ForwarderDeployerLibrary",
231-
SuperfluidERC2771ForwarderDeployerLibraryArtifact,
232-
signer
233-
);
234227

235228
const sfDeployer = await _getFactoryAndReturnDeployedContract(
236229
"SuperfluidFrameworkDeployer",
@@ -276,9 +269,6 @@ const _deployTestFramework = async (provider, signer) => {
276269
SuperTokenFactoryDeployerLibrary
277270
),
278271
TokenDeployerLibrary: getContractAddress(TokenDeployerLibrary),
279-
SuperfluidERC2771ForwarderDeployerLibrary: getContractAddress(
280-
SuperfluidERC2771ForwarderDeployerLibrary
281-
),
282272
},
283273
}
284274
);

packages/ethereum-contracts/ops-scripts/deploy-framework.js

Lines changed: 64 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
232232
"PoolAdminNFT",
233233
"PoolMemberNFT",
234234
"IAccessControlEnumerable",
235+
"SimpleForwarder",
235236
"ERC2771Forwarder",
236237
];
237238
const mockContracts = [
@@ -270,6 +271,7 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
270271
PoolAdminNFT,
271272
PoolMemberNFT,
272273
IAccessControlEnumerable,
274+
SimpleForwarder,
273275
ERC2771Forwarder,
274276
} = await SuperfluidSDK.loadContracts({
275277
...extractWeb3Options(options),
@@ -351,6 +353,10 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
351353
`Superfluid.${protocolReleaseVersion}`,
352354
async (contractAddress) => !(await hasCode(web3, contractAddress)),
353355
async () => {
356+
const simpleForwarder = await web3tx(SimpleForwarder.new, "SimpleForwarder.new")();
357+
console.log("SimpleForwarder address:", simpleForwarder.address);
358+
output += `SIMPLE_FORWARDER=${simpleForwarder.address}\n`;
359+
354360
const erc2771Forwarder = await web3tx(ERC2771Forwarder.new, "ERC2771Forwarder.new")();
355361
console.log("ERC2771Forwarder address:", erc2771Forwarder.address);
356362
output += `ERC2771_FORWARDER=${erc2771Forwarder.address}\n`;
@@ -359,15 +365,12 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
359365
const superfluidLogic = await web3tx(
360366
SuperfluidLogic.new,
361367
"SuperfluidLogic.new"
362-
)(nonUpgradable, appWhiteListing, appCallbackGasLimit, erc2771Forwarder.address);
368+
)(nonUpgradable, appWhiteListing, appCallbackGasLimit, simpleForwarder.address, erc2771Forwarder.address);
363369
console.log(
364370
`Superfluid new code address ${superfluidLogic.address}`
365371
);
366372
output += `SUPERFLUID_HOST_LOGIC=${superfluidLogic.address}\n`;
367-
// get the address of SimpleForwarder (deployed in constructor) for verification
368-
const simpleForwarderAddr = await superfluidLogic.SIMPLE_FORWARDER();
369-
console.log("SimpleForwarder address", simpleForwarderAddr);
370-
output += `SIMPLE_FORWARDER=${simpleForwarderAddr}\n`;
373+
371374
if (!nonUpgradable) {
372375
const proxy = await web3tx(
373376
UUPSProxy.new,
@@ -383,10 +386,15 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
383386
superfluidAddress = superfluidLogic.address;
384387
}
385388
const superfluid = await Superfluid.at(superfluidAddress);
389+
await web3tx(
390+
simpleForwarder.transferOwnership,
391+
"simpleForwarder.transferOwnership"
392+
)(superfluid.address);
386393
await web3tx(
387394
erc2771Forwarder.transferOwnership,
388395
"erc2771Forwarder.transferOwnership"
389396
)(superfluid.address);
397+
390398
await web3tx(
391399
superfluid.initialize,
392400
"Superfluid.initialize"
@@ -784,33 +792,60 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
784792
throw new Error("Superfluid is not upgradable");
785793
}
786794

787-
async function getPrevERC2771ForwarderAddr() {
788-
try {
789-
return await superfluid.getERC2771Forwarder();
790-
} catch (err) {
791-
console.error("### Error getting ERC2771Forwarder address, likely not yet deployed");
792-
return ZERO_ADDRESS; // fallback
795+
async function getOrDeployForwarder(
796+
superfluid,
797+
ForwarderContract,
798+
getPrevAddrFn,
799+
outputKey
800+
) {
801+
let prevAddr = await getPrevAddrFn().catch(_err => {
802+
console.error(`### Error getting ${ForwarderContract.contractName} address, likely not yet deployed`);
803+
return ZERO_ADDRESS;
804+
});
805+
806+
{
807+
// TEMPORARY FIX - can be removed after applied
808+
// we found a previous deployment. Now verify it has the host as owner.
809+
// the first mainnet deployment didn't have this for SimpleForwarder, thus needs a redeployment.
810+
const ownerAddr = await (await Ownable.at(prevAddr)).owner();
811+
if (ownerAddr != superfluid.address) {
812+
console.log(` !!! ${outputKey} has wrong owner, needs re-deployment`);
813+
prevAddr = ZERO_ADDRESS; // by setting zero, we force a re-deployment
814+
}
793815
}
816+
817+
const newAddress = await deployContractIfCodeChanged(
818+
web3,
819+
ForwarderContract,
820+
prevAddr,
821+
async () => {
822+
const forwarder = await web3tx(ForwarderContract.new, `${ForwarderContract.contractName}.new`)();
823+
await web3tx(
824+
forwarder.transferOwnership,
825+
"forwarder.transferOwnership"
826+
)(superfluid.address);
827+
828+
output += `${outputKey}=${forwarder.address}\n`;
829+
return forwarder.address;
830+
}
831+
);
832+
833+
return newAddress !== ZERO_ADDRESS ? newAddress : prevAddr;
794834
}
795-
const prevERC2771ForwarderAddr = await getPrevERC2771ForwarderAddr();
796835

797-
const erc2771ForwarderNewAddress = await deployContractIfCodeChanged(
798-
web3,
836+
const simpleForwarderAddress = await getOrDeployForwarder(
837+
superfluid,
838+
SimpleForwarder,
839+
() => superfluid.SIMPLE_FORWARDER(),
840+
"SIMPLE_FORWARDER"
841+
);
842+
843+
const erc2771ForwarderAddress = await getOrDeployForwarder(
844+
superfluid,
799845
ERC2771Forwarder,
800-
prevERC2771ForwarderAddr,
801-
async () => {
802-
const erc2771Forwarder = await web3tx(ERC2771Forwarder.new, "ERC2771Forwarder.new")();
803-
await web3tx(
804-
erc2771Forwarder.transferOwnership,
805-
"erc2771Forwarder.transferOwnership"
806-
)(superfluid.address);
807-
output += `ERC2771_FORWARDER=${erc2771Forwarder.address}\n`;
808-
return erc2771Forwarder.address;
809-
}
846+
() => superfluid.getERC2771Forwarder(),
847+
"ERC2771_FORWARDER"
810848
);
811-
const erc2771ForwarderAddress = erc2771ForwarderNewAddress !== ZERO_ADDRESS
812-
? erc2771ForwarderNewAddress
813-
: prevERC2771ForwarderAddr;
814849

815850
// get previous callback gas limit, make sure we don't decrease it
816851
const prevCallbackGasLimit = await superfluid.CALLBACK_GAS_LIMIT();
@@ -820,13 +855,6 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
820855
console.log(` !!! CHANGING APP CALLBACK GAS LIMIT FROM ${prevCallbackGasLimit} to ${appCallbackGasLimit} !!!`);
821856
}
822857

823-
// get prev SimpleForwarder addr (only for replacements for codeChanged check)
824-
let simpleForwarderAddr;
825-
try {
826-
simpleForwarderAddr = await superfluid.SIMPLE_FORWARDER();
827-
} catch (error) {
828-
simpleForwarderAddr = ZERO_ADDRESS;
829-
}
830858
// deploy new superfluid host logic
831859
superfluidNewLogicAddress = await deployContractIfCodeChanged(
832860
web3,
@@ -839,13 +867,13 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
839867
const superfluidLogic = await web3tx(
840868
SuperfluidLogic.new,
841869
"SuperfluidLogic.new"
842-
)(nonUpgradable, appWhiteListing, appCallbackGasLimit, erc2771ForwarderAddress);
870+
)(nonUpgradable, appWhiteListing, appCallbackGasLimit, simpleForwarderAddress, erc2771ForwarderAddress);
843871
output += `SUPERFLUID_HOST_LOGIC=${superfluidLogic.address}\n`;
844872
return superfluidLogic.address;
845873
},
846874
[
847875
ap(erc2771ForwarderAddress),
848-
ap(simpleForwarderAddr),
876+
ap(simpleForwarderAddress),
849877
appCallbackGasLimit.toString(16).padStart(64, "0")
850878
],
851879
);

0 commit comments

Comments
 (0)