Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove TransferWhitelist contract #10318

Merged
merged 5 commits into from
Jun 9, 2023
Merged

Conversation

karlb
Copy link
Contributor

@karlb karlb commented May 17, 2023

The contract is not used anymore. See also celo-org/celo-blockchain#2110.

@karlb karlb force-pushed the karlb/remove-transfer-whitelist branch from 20662ce to 38705af Compare May 17, 2023 12:23
@karlb karlb marked this pull request as ready for review May 19, 2023 08:37
@karlb karlb requested review from a team as code owners May 19, 2023 08:37
@karlb karlb requested a review from a team May 19, 2023 08:37
@martinvol
Copy link
Contributor

I think it's worth noticing that even if we delete the contract itself, our on-chain contract Registry does not have a function to remove, so the deployed implementation will keep being a core-contract.

I do not see a downside with deleting the contracts, as it is already verified in Celo Explorer: https://explorer.celo.org/mainnet/address/0xb49E4d6F0B7f8d0440F75697E6c8b37E09178BCF/contracts#address-tabs

Copy link
Contributor

@eelanagaraj eelanagaraj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good other than the comment + the GrandaMento CK test failure, which like it's probably caused by the stableToken contract address now being migrated one contract earlier than before ( ithout the transferWhiteList contract migration happening)

packages/protocol/migrationsConfig.js Show resolved Hide resolved
Copy link
Contributor

@eelanagaraj eelanagaraj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀
edit: flagging that the build is failing, would double check that but once this is updated + build is passing, good to go 👍

@karlb
Copy link
Contributor Author

karlb commented May 31, 2023

flagging that the build is failing, would double check that

The current build failures are present in master. See

I can wait until master is green again and rebase then to get the the tests green.

karlb added a commit that referenced this pull request May 31, 2023
since it is possible that someone will try to verify previous releases
and since this contract is part of previous releases it might happen
that verification will fail (or we should double check that it will not
fail).

See
#10318 (comment).
@karlb karlb force-pushed the karlb/remove-transfer-whitelist branch from 02357ab to b2ab3c3 Compare May 31, 2023 11:51
Copy link
Contributor

@soloseng soloseng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

karlb added a commit that referenced this pull request Jun 9, 2023
since it is possible that someone will try to verify previous releases
and since this contract is part of previous releases it might happen
that verification will fail (or we should double check that it will not
fail).

See
#10318 (comment).
@karlb karlb force-pushed the karlb/remove-transfer-whitelist branch from b2ab3c3 to 3694a4d Compare June 9, 2023 11:19
karlb added 5 commits June 9, 2023 13:48
The contract is not used anymore. See also
celo-org/celo-blockchain#2110.
Otherwise the test fails when one of the migrations before the cUSD
deployment changes.
since it is possible that someone will try to verify previous releases
and since this contract is part of previous releases it might happen
that verification will fail (or we should double check that it will not
fail).

See
#10318 (comment).
@karlb karlb force-pushed the karlb/remove-transfer-whitelist branch from 3694a4d to 1f5920d Compare June 9, 2023 11:48
@karlb karlb enabled auto-merge (squash) June 9, 2023 11:58
@karlb karlb merged commit a8d51e6 into master Jun 9, 2023
@karlb karlb deleted the karlb/remove-transfer-whitelist branch June 9, 2023 12:18
@martinvol martinvol mentioned this pull request Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants