Emit Event for initializer and saltNonce data

[remedcu]

As part of improving the capabilities for indexing safe creation data, a new set of functions is introduced which emits an extra event named ProxyCreationL2 which also emits the initializer and saltNonce. The previous function remains the same for backward compatibility.

[mmv08]

1. How much additional guess does it consume? In other words, does the difference justify an extra function?
2. Personal opinion: Our previous L2 naming created a lot of confusion for the developers, who thought that such contracts should be used exclusively on L2s. I'd suggest the naming to be more descriptive of what's actually happening: e.g., createProxyWithNonceEmitExtraEvent

[remedcu]

1. How much additional guess does it consume?

I haven't checked, but the gas cost will be dynamic based on the bytes initializer data. With an extra event (375), the indexed proxy (375) and bytes initializer (8 * data_size) contributing to the most amount of gas used.

In other words, does the difference justify an extra function?

The extra function is to keep the backward compatibility with the previous method and for networks which doesn't require this extra event for decoding the safe creation.

2. Personal opinion: Our previous L2 naming created a lot of confusion for the developers, who thought that such contracts should be used exclusively on L2s. I'd suggest the naming to be more descriptive of what's actually happening: e.g., createProxyWithNonceEmitExtraEvent

I followed the same convention as we used SafeL2 for the previous name with the extra event. Could change to createProxyWithNonceAndEvent to keep it descriptive but short.

[nlordell]

I suggested the L2 naming so that it matches the same convention as SafeL2.

In other words, does the difference justify an extra function?

I think there is a non-trivial amount of gas to emit an additional event (we don't want to replace the event for compatibility reasons). This event is only needed on chains without tracing support (i.e. same compatibility concerns with Safe vs SafeL2) so it feels bad to emit two events on Mainnet for example.

@remedcu - can you exactly quantify the difference in gas units for calling createProxyWithNonce with a "regular" Safe (i.e. 1/1 with compatibility fallback handler and no setup call)? Then we can get exact numbers and make a better decision.

[mmv08]

I suggested the L2 naming so that it matches the same convention as SafeL2.

In my opinion, I would not stick to this convention as it confuses people and is just incorrect. We use SafeL2 on gnosis chain, polygon, aurora, bnb, avalanche, but they're not L2s.

But I also see the argument for sticking to the convention, so it's not a blocker.

[remedcu]

@remedcu - can you exactly quantify the difference in gas units for calling createProxyWithNonce with a "regular" Safe (i.e. 1/1 with compatibility fallback handler and no setup call)? Then we can get exact numbers and make a better decision.

Based on some changes to the benchmark test, these are the results:

createProxyWithNonce(...)

➜  safe-contracts git:(safe-proxy-event) ✗ npx hardhat test benchmark/Safe.Creation.spec.ts
  Safe
           Used 114695n gas for >Setup Safe with 1 owner(s) and fallback handler<

createProxyWithNonceL2(...)

➜  safe-contracts git:(safe-proxy-event) ✗ npx hardhat test benchmark/Safe.Creation.spec.ts
  Safe
           Used 117165n gas for >Setup Safe with 1 owner(s) and fallback handler<

So, 2470 gas per creation extra without setup call (initializer as 0x)

[nlordell]

So, I was wondering about a different benchmark 😅:

1. Change createProxyWithNonce to emit both events (both are needed for backwards compatibility)
2. Set initializer to be setup([owner], 1, ...) - so a typical Safe setup.

And compare this with the current cost of a createProxyWithNonce call.

[remedcu]

Current createProxyWithNonce(...)

➜  safe-contracts git:(safe-proxy-event) ✗ npx hardhat test benchmark/Safe.Creation.spec.ts
  Safe
           Used 255938n gas for >Setup Safe with 1 owner(s) and fallback handler<
           Used 255938n gas for >Setup Safe with 1 owner(s) and fallback handler<
           Used 279396n gas for >Setup Safe with 2 owner(s) and fallback handler<
           Used 302843n gas for >Setup Safe with 3 owner(s) and fallback handler<
           Used 349759n gas for >Setup Safe with 5 owner(s) and fallback handler<

New createProxyWithNonce(...) with extra event emitted.

➜  safe-contracts git:(safe-proxy-event) ✗ npx hardhat test benchmark/Safe.Creation.spec.ts
  Safe
           Used 262484n gas for >Setup Safe with 1 owner(s) and fallback handler< // 6546 (2.49% higher)
           Used 262484n gas for >Setup Safe with 1 owner(s) and fallback handler< // 6546 (2.49% higher)
           Used 286276n gas for >Setup Safe with 2 owner(s) and fallback handler< // 6880 (2.4% higher)
           Used 310057n gas for >Setup Safe with 3 owner(s) and fallback handler< // 7214 (2.32% higher)
           Used 357642n gas for >Setup Safe with 5 owner(s) and fallback handler< // 7883 (2.2% higher)

createProxyWithNonceL2(...) (52 gas extra for the call to createProxyWithNonce(...))

➜  safe-contracts git:(safe-proxy-event) ✗ npx hardhat test benchmark/Safe.Creation.spec.ts
  Safe
           Used 262536n gas for >Setup Safe with 1 owner(s) and fallback handler<
           Used 262536n gas for >Setup Safe with 1 owner(s) and fallback handler<
           Used 286328n gas for >Setup Safe with 2 owner(s) and fallback handler<
           Used 310109n gas for >Setup Safe with 3 owner(s) and fallback handler<
           Used 357694n gas for >Setup Safe with 5 owner(s) and fallback handler<

[nlordell]

Thanks for the detailed analysis! Personally, I like how it was implemented here (with the double event emitted for extra compatibility!).