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

add: supertoken protocol nft upgrade + celo gas token #269

Merged
merged 20 commits into from
Oct 9, 2023

Conversation

sirpy
Copy link
Contributor

@sirpy sirpy commented Mar 22, 2023

Description

  • Add compatibility with latest superfluid NFT upgrade
  • Add debitGasFees and creditGasFees for Celo gas token compatability
  • code size optimizations: removed unused functions, moved modifiers into inline function calls

See superfluid NFT upgrade:
superfluid-finance/protocol-monorepo#1274

  • NOTICE: superfluid added the deployment logic of the new NFTs inside the supertoken logic. Due to contract size issues, we assume that the NFTs will be deployed outside supergooddollar and they could be set using the setNFTProxyContracts method allowed only by the Avatar.
  • only relevant contract in the PR is supertoken.sol we also had to change isupertoken.sol to remove unused methods.

See Celo instructions for gas token compatability:

@sirpy sirpy marked this pull request as draft March 22, 2023 08:36
@0xdavinchee
Copy link

Other than the comments I mentioned, everything else looks in order! new storage is in the correct slots and other logic related to the NFT contracts have been properly added

@sirpy sirpy changed the title add: supertoken protocol nft upgrade add: supertoken protocol nft upgrade + celo gas token Mar 24, 2023
@sirpy sirpy requested a review from 0xdavinchee March 24, 2023 09:09
Copy link

@0xdavinchee 0xdavinchee left a comment

Choose a reason for hiding this comment

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

The changes you've made here in the SuperGoodDollar and SuperToken contracts as a result of the new Flow NFT feature and v1.5.2 of ethereum-contracts ensures that SuperGoodDollar has access to these new features.

@sirpy sirpy marked this pull request as ready for review March 27, 2023 08:01
address from,
address feeRecipient,
address gatewayFeeRecipient,
address communityFund,

Choose a reason for hiding this comment

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

The name of this parameter is kinda legacy as fees will soon not be sent to the community fund, so maybe we can update that to baseFeeAddress or something like that.

@martinvol
Copy link

Took a look at the code and from a high level view, it seems like they do what it's expected from them. We could try to deploy this to an staging testnet to see how it behaves.

@sirpy
Copy link
Contributor Author

sirpy commented Jul 2, 2023

@martinvol how can we deploy that to alfajores?

@sirpy sirpy merged commit 38645ad into master Oct 9, 2023
23 checks passed
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.

3 participants