bancorprotocol / contracts-solidity

Bancor Protocol Contracts
Other
840 stars 391 forks source link

Remove the whitelist contract #622

Closed lbeder closed 3 years ago

lbeder commented 3 years ago

@barakman I don't see it there. Where is it?

barakman commented 3 years ago

@barakman I don't see it there. Where is it?

https://github.com/bancorprotocol/contracts-solidity/blob/3.0.0/utils/test_deployment.js#L416

lbeder commented 3 years ago

@barakman that's a completely different whitelist... this PR removes the no longer used Whitelist contract

barakman commented 3 years ago

@barakman that's a completely different whitelist... this PR removes the no longer used Whitelist contract

Don't we want to remove all whitelist-related aspects in the system?

lbeder commented 3 years ago

@barakman that's a completely different whitelist... this PR removes the no longer used Whitelist contract

Don't we want to remove all whitelist-related aspects in the system?

Not that I'm aware of, but even if it was the case - how is it related to this PR?

barakman commented 3 years ago

@barakman that's a completely different whitelist... this PR removes the no longer used Whitelist contract

Don't we want to remove all whitelist-related aspects in the system?

Not that I'm aware of, but even if it was the case - how is it related to this PR?

In the sense that we may want to consider the primary goal of this PR - whether it is the technical goal of removing a specific contract, or the conceptual goal of removing an entire aspect of the system.

There is obviously no point in keeping whitelisted-related functionality once the whitelist contract is gone.

lbeder commented 3 years ago

There is only a single purpose to this PR and it's to remove a no longer used contract (a separate PR was opened to add it to the legacy repository). That's it. Current LP whitelisting functionality is implemented using the EnumerableSet.AddressSet primitive.