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

perf: optimize NFTDescriptor contract #1113

Merged
merged 4 commits into from
Dec 12, 2024
Merged

perf: optimize NFTDescriptor contract #1113

merged 4 commits into from
Dec 12, 2024

Conversation

smol-ninja
Copy link
Member

@smol-ninja smol-ninja commented Dec 11, 2024

Changelog

@andreivladbrg this PR is built on top of #1108. There was an another issue due to which tokenURI was returning ERC20 instead of DAI (comment). All issues have been fixed in this PR.

I recommend that we close #1108 in favour of this PR. I have also added you as the co-author in this.

Previews

Click to see result of tokenURI
{
  "attributes": [
    {
      "trait_type": "Token",
      "value": "DAI"
    },
    {
      "trait_type": "Sender",
      "value": "0x6332e7b1deb1f1a0b77b2bb18b144330c7291bca"
    },
    {
      "trait_type": "Status",
      "value": "Streaming"
    }
  ],
  "description": "This NFT represents a stream in Sablier Lockup contract. The owner of this NFT can withdraw the streamed tokens, which are denominated in DAI.\\n\\n- Stream ID: 1\\n- Sablier Lockup Address: 0x923b5ab3714fd343316af5a5434582fd16722523\\n- DAI Address: 0xf62849f9a0b5bf2913b396098f7c7019b51a820a\\n\\n⚠️ WARNING: Transferring the NFT makes the new owner the recipient of the stream. The funds are not automatically withdrawn for the previous recipient.",
  "external_url": "https://sablier.com",
  "name": "Sablier Lockup #1",
  "image": "see image below"
Click to expand SVG preview Screenshot 2024-12-11 at 15 20 53
Click to preview description on Opensea
This NFT represents a stream in Sablier Lockup contract. The owner of this NFT can withdraw the streamed tokens, which are denominated in DAI.
    Stream ID: 1
    Sablier Lockup Address: 0x923b5ab3714fd343316af5a5434582fd16722523
    DAI Address: 0xf62849f9a0b5bf2913b396098f7c7019b51a820a
    ⚠️ WARNING: Transferring the NFT makes the new owner the recipient of the stream. The funds are not automatically withdrawn for the previous recipient.

Copy link
Member

@andreivladbrg andreivladbrg left a comment

Choose a reason for hiding this comment

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

@smol-ninja smol-ninja merged commit b3ea122 into staging Dec 12, 2024
9 checks passed
@smol-ninja smol-ninja deleted the fix/tokenURI branch December 12, 2024 07:49
andreivladbrg added a commit that referenced this pull request Jan 27, 2025
* test: update token uri tests

* test: remove tokenURI tests for each model

* perf: optimize NFTDescriptor contract
feat: remove backward compatibility

Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>

* perf: remove no longer needed low level call

---------

Co-authored-by: andreivladbrg <[email protected]>
Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>
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.

2 participants