Remove TokenTimelock, PaymentSplitter, ERC20Snapshot, ERC20VotesComp, GovernorVotesComp

[Amxx]

Fixes LIB-874

PR Checklist

• Tests
• Documentation
• Changeset entry (run npx changeset add)

[changeset-bot]

⚠️ No Changeset found

Latest commit: e51e218

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[socket-security]

No dependency changes detected. Learn more about Socket for GitHub ↗︎

---

👍 No dependency changes detected in pull request

Pull request alert summary

Issue	Status
Critical CVE	✅ 0 issues
CVE	✅ 0 issues
Mild CVE	✅ 0 issues
Install scripts	✅ 0 issues
Native code	✅ 0 issues
Bin script shell injection	✅ 0 issues
Unresolved require	✅ 0 issues
Invalid package.json	✅ 0 issues
HTTP dependency	✅ 0 issues
Git dependency	✅ 0 issues
GitHub dependency	✅ 0 issues
Potential typo squat	✅ 0 issues
Known Malware	✅ 0 issues
Telemetry	✅ 0 issues
Protestware/Troll package	✅ 0 issues
AI detected security risk	✅ 0 issues
AI warning	✅ 0 issues

[frangio]

Upgradeable patch needs to be updated.

[ItsCuzzo]

Hey there, out of curiosity what was the rationale behind the removal of PaymentSplitter? It was quite a useful contract that I have utilised a variety of times during development, particularly with ERC-721 token contracts with multiple shareholders.

[Amxx]

Hello @ItsCuzzo

The PaymentSplitter received a lot of criticism related to the inability to update the share. Some people would expect the shares to be transferable using an ERC20 or ERC721 interface.

The plan is to remove the old version (that suffers limitations) in version 5.0, and then reintroduce a better version down the line in 5.1 or 5.2. In the meantime, the old version remains available in the 4.X versions.

[ItsCuzzo]

Awesome, I did see some of that discussion now that you mention it. Thanks for the detailed reply!

[Arvolear]

Hey, is it the same story with ERC20Snapshot? It was indeed ultra useful for some lightweight DAOs.

[frangio]

Please say more about "ultra lightweight"?

[Arvolear]

I mean DAOs with ERC20Snapshot as a voting token.

[Amxx]

ERC20Snapshot does not support delegation, and requires someone to create a snapshot when needed (part of the proposal workflow I guess).

This doesn't sound like the best design.

[Arvolear]

There are many approaches to build DAOs and I agree that the absence of delegation might break some design decision.

But there is one case with 2 step DAOs. In these DAOs there is a set of validators that is elected by regular members to veto proposals.

These validators should not delegate and they have fixed voting power. Snapshots here fix the problem when there are 2 proposals (regular and to remove a validator).

Let's say the regular one lasts for 2 weeks and the removal is only 1 week long.

So if validator gets removed, he won't be able to vote on the regular proposal even though it was created first.

[Amxx]

To add more context:

• ERC20Snapshot was released before ERC20Votes, it was a first approach.
• When ERC20Votes was released it was considered the better option. In particular becauseits a more standard interface that is supported by some third party tools (such as Tally)
• Our governor is compatible with ERC20Votes (and ERC721Votes), but not with ERC20Snapshot.

Planning for 5.0, we felt that the Votes module was the go to solution, and that ERC20Snapshot was no longer relevant. We decided to deprecate it, and it is now removed for the repo. It remains available in the 4.x packages.

If you really want, you should be able to adapt ERC20Snapshot to 5.0 easily.

[ernestognw]

We got similar questions quite a few times and I think we need to provide a bit more clarity on how to replace ERC20Snapshot. Although I agree with @Amxx in that people should generally use ERC20Votes for governance (idk how the "lightweight" DAO case works), it's true there are use cases for snapshotting that aren't related to governance and the alternatives aren't trivial:

• On one hand, you can use the 4.x version of ERC20Snapshot, but if you mix it with 5.x contracts, they wont be guaranteed to have compatible storage and semantics with 4.x. In a new minor version, we may add new storage variables or slightly modify the semantics of a function in a way that mixed with a 4.x's ERC20Snapshot could break some of your assumptions.
• On the other, you can use a modification of ERC20Votes to mimic the ERC20Snapshot behavior, but I don't think that's as easy as it sounds, and requires a bit of context.

I'd generally go for adapting an ERC20Votes because it guarantees support during the 5.x version, but given that the contract is built for governance purposes, I see there are a few important differences to ERC20Snapshot:

• The "snapshots" (now called checkpoints) depend on the EIP-6372 contract clock, as opposed to an snapshotId.
• The Votes contract counts delegated tokens (by default to the address 0)

So the ERC20Votes should be overridden to use a counter as clock() and have self-delegation by default. Here's a PoC
for reference purposes, note its unaudited code.

Also, an ERC20Snapshot implemented from scratch is also relatively easy using the OpenZeppelin Checkpoints primitive. Here's a reference PoC. Unaudited as well.