1inch Swap-audit met vaste rente

Bronknooppunt: 1609993

Introductie

De 1inch team vroeg ons om hun slimme contracten met Fixed Rate Swap te beoordelen en te controleren. We hebben de code bekeken en publiceren nu onze resultaten.

strekking

We hebben commit gecontroleerd b1600f61b77b6051388e6fb2cb0be776c5bcf2d1 van de 1inch/fixed-rate-swap opslagplaats. In scope was het volgende contract:

- FixedRateSwap.sol

Alle andere projectbestanden en mappen, inclusief tests, externe afhankelijkheden en projecten, speltheorie en incentive-ontwerp, werden uitgesloten van de reikwijdte van deze audit. Externe code en contractafhankelijkheden werden verondersteld te werken zoals gedocumenteerd.

Algemene gezondheid

We ontdekten dat de codebase complex was en dat er vaak onvoldoende documentatie was om de niet-triviale wiskunde en logica die overal gebruikt werd uit te leggen. Voor een codebase van deze omvang beschouwen we het enorme aantal problemen in dit rapport als indicatief voor onnodige complexiteit. Samen met een gebrek aan documentatie, is de codebase niet op het niveau van verfijning dat we zouden verwachten van een productieklaar systeem. Het elimineren van de, zij het slimme, continue vergoedingsfunctie en het vervangen ervan door drie afzonderlijke lineaire functies zou veel van de codebase veel gemakkelijker kunnen maken om over te redeneren. Een eenvoudiger ontwerp zou veel van de meest complexe codeblokken kunnen elimineren waar problemen werden geïdentificeerd. Refactoring en vereenvoudiging van het protocol is ten zeerste aan te raden, in welk geval we aanbevelen om aanvullende audits te ondergaan.

Al met al stond het 1inch-team altijd klaar om eventuele vragen te beantwoorden tijdens de audit en ze waren erg gemakkelijk om mee samen te werken. Ze staan ​​open voor feedback en suggesties voor continue verbetering.

Overwegingen bij gasverbruik

Er zijn enkele bijzonder gasintensieve delen van de codebase, zoals de _powerHelper functie en binaire zoekopdrachten, die we hebben waargenomen met meer dan 400k gas in sommige randgevallen. Gezien de aard van het protocol en de stabiele munten waarmee het van plan is om te gaan, kan het hoge gasverbruik dat de complexiteit van het project met zich meebrengt, het doel van het protocol ondermijnen en zeker de algemene bruikbaarheid ervan beïnvloeden.

Systeem overzicht

Het Fixed Rate Swap-protocol is een Automatic Market Maker (AMM) die een constante prijscurve gebruikt om swaps tussen activa waarvan de prijzen stabiel zijn ten opzichte van elkaar te vergemakkelijken.
Het implementeert ook een variabel vergoedingsmechanisme dat: algemeen rekent 0.01% als vergoedingen. Wanneer de tokensaldi van de AMM extreme omstandigheden bereiken, worden de vergoedingen ofwel verlaagd tot 0% of verhoogd tot 0.2%, afhankelijk van of de activiteit wenselijk is of niet, respectievelijk, voor de samenstelling van de activa van de pool. Dergelijke vergoedingen worden bewaard in de pool, waar liquiditeitsverschaffers ervan zullen profiteren door de aandelen (die van de AMM) aan te houden ERC20 LP-token).

Bevoorrechte rollen

Er zijn geen bevoorrechte rollen in de gecontroleerde versie van het protocol.

Externe afhankelijkheden en vertrouwensaannames

Het protocol is niet bedoeld om te ondersteunen ERC20 tokens die kosten in rekening brengen bij overdracht.

Het is ook vermeldenswaard dat er activa kunnen zijn die, afhankelijk van hun aard, onverwachte gedragingen kunnen veroorzaken die vergelijkbaar zijn met die welke in dit rapport worden beschreven, bijv. stabiele munten met een onlogisch aantal decimalen tussen elkaar of activa die onbeperkte flitsleningen mogelijk maken .

Bevindingen

Hier presenteren we onze bevindingen.

update: Het 1inch-team heeft verschillende fixes toegepast op basis van onze aanbevelingen en heeft ons een reeks commits gegeven die gericht zijn op de respectievelijke gevonden problemen. Deze commits worden vermeld bij elk respectief probleem en we behandelen de fixes die onder elke individuele commit zijn geïntroduceerd. We hebben echter alleen specifieke patches beoordeeld voor de problemen die we hebben gemeld. De codebase heeft enkele andere wijzigingen ondergaan die we niet hebben gecontroleerd en we raden aan die wijzigingen in een toekomstige audit grondig te bekijken.

Kritieke ernst

Geen.

Hoge ernst

Geen.

Middelmatige ernst

[M01] Gebrek aan outputgaranties kan leiden tot verlies van middelen

De FixedRateSwap contract faciliteert de swapping van de ene stabiele munt voor de andere, en stelt gebruikers ook in staat om storting en intrekken activa voor "aandelen" van de liquiditeitspool (LP-tokens).

Noch swaps, opnames of deposito's bieden gebruikers echter een mechanisme om minimaal aanvaardbare rendementswaarden voor interacties met de pool in te stellen.

De pool beoordeelt variabele vergoedingen op basis van hoe interacties de samenstelling van de pool beïnvloeden. Desalniettemin is er met dit ontwerp, aangezien beoordeelde vergoedingen rechtstreeks naar poolaandeelhouders zouden gaan, een economische prikkel voor de meerderheidsaandeelhouder om grote token-swaps te leiden, waarbij de samenstelling van de pool wordt gemanipuleerd om hogere vergoedingen af ​​te dwingen.

Evenzo kan front-running ook worden gebruikt om te profiteren van protocolafronding binnen token-uitvoerberekeningen. Hoewel onwaarschijnlijk gezien de huidige staat van het ecosysteem met betrekking tot populaire stabiele munten, als de fromBalance van een token in de pool kan worden gemanipuleerd om in de volgorde te staan ​​van 1e36 * inputAmount voor een bepaalde swap, dan deze lijn van de _getReturn functie zou worden afgekapt tot zero, wat leidt tot een nulwaarde outputAmount.

Echter, omdat nulwaarde-swaps zijn beschermd tegen:, afknotting binnen de outputAmount berekening zou plaatsvinden vóór de invoer in het slechtste geval. in het bijzonder, een fromBalance overal binnen het bereik van [1e26 naar 1e35] * the input amount kan leiden tot inkorting en een vermindering van de outputAmount die winstgevend kunnen zijn voor poolaandeelhouders ten koste van gebruikers.

Hoewel een dergelijk scenario momenteel onwaarschijnlijk is, kunnen toekomstige veranderingen in het ecosysteem en onbeperkte flitsleningen dit een meer levensvatbare aanvalsvector maken.

Om onbedoeld verlies van geld te voorkomen en mogelijke front-running aanvallen te beperken, kunt u overwegen gebruikers toe te staan ​​om de minimaal aanvaardbare retourwaarde te specificeren voor elke interactie met het protocol die financiële implicaties heeft.

update: Vast in plegen ea75a86. Er is echter geen specifieke test toegevoegd om te valideren dat de implementatie van de fix deze aanval voorkomt.

Lage ernst

[L01] Aanbetaling kan terugkeren bij onderstroom

Als een pool dramatisch uit balans is, zullen de deposito's terugkeren tijdens de berekening van de shift variabele in de _getVirtualAmountsForDepositImpl functie tenzij het totale gestorte bedrag groter is dan 1/1000ste van het bestaande saldo in de pool.

Als u bijvoorbeeld probeert een verhouding van tokens van in totaal minder dan 10 te storten in een pool met bestaande saldi van 10,000 token0 en .0001 token1, zal terug draaien. Het storten van een groter bedrag zal slagen.

Dit kan worden gebruikt om een ​​barrière te creëren om te voorkomen dat andere gebruikers zich bij de pool aansluiten. In een dergelijk scenario is het nog steeds mogelijk om tokens op te nemen en te ruilen via de pool. Het is ook mogelijk om in exact dezelfde verhouding van het zwembad te storten. Verschuift de pool naar minder eenzijdig, dan worden kleinere stortingen weer mogelijk.

Om de kans op denial-of-service-aanvallen te voorkomen, kunt u overwegen de codebase te herzien om deze edge-cases beter op te vangen. Overweeg op zijn minst expliciet te controleren op deze voorwaarden en nuttige foutmeldingen te retourneren bij het terugzetten.

update: Vast in plegen 4aa5210.

[L02] Onvolledige gebeurtenisemissies

De Swap gebeurtenis wordt uitgezonden elke keer dat het protocol een token-swap mogelijk maakt.

Er zijn echter verschillende openbare methoden beschikbaar om token-swaps uit te voeren. De swap0To1 en swap1To0 functies sturen het resultaat van de swap direct naar de msg.sender, Maar de swap0To1For en swap1To0For functies sturen de resultaten van de swap naar een adres dat expliciet is opgegeven door de to parameter.

Sinds dezelfde gebeurtenis die het in beide gevallen heeft uitgezonden, hebben off-chain partijen geen manier om te onderscheiden welk soort swap is uitgevoerd of aan wie het resultaat van de swap is geleverd.

Evenzo is de Deposit en Withdrawal evenementen accepteren slechts een enkele user adresparameter, hoewel ze ook worden uitgezonden in functies waarbij de msg.sender kan anders zijn dan de partij die tokens ontvangt.

Overweeg een geïndexeerde recipient adresparameter aan de gebeurtenissen, zodat ze vollediger kunnen overbrengen welke acties worden ondernomen door het protocol.

update: Erkend, en zal niet repareren.

[L03] Gebrek aan invoervalidatie

De public getReturn functie ontbreekt enige invoervalidatie. specifiek:

  • Het bevestigt niet dat de tokenFrom en tokenTo adressen zijn de tokens die de pool vormen.
  • Het bevestigt niet dat de inputAmount parameter is niet nul.
  • Het bevestigt niet dat de fromBalance + toBalance waarde is niet nul.

Bovendien, noch de withdrawFor noch withdrawForWithRatio functies berekenen of een gebruiker genoeg aandelen heeft om het gevraagde bedrag op te nemen. In beide gevallen wordt een opname teruggedraaid als niet aan de voorwaarde is voldaan, maar de foutmeldingen zijn onduidelijk en worden alleen afgegeven na het verbruiken van onnodig gas.

Ten slotte, de withdrawForWithRatio functie verzuimt ervoor te zorgen dat de waarden die worden gebruikt voor virtuele balansberekeningen kleiner zijn dan of gelijk zijn aan de werkelijke saldi van het contract. Een dergelijk scenario zou leiden tot een cryptische rekenkundige terugkeer bij het berekenen balanceX en balanceY voor de _getRealAmountsForWithdrawImpl functie.

Een gebrek aan validatie van door de gebruiker gecontroleerde parameters kan leiden tot foutieve of falende transacties die moeilijk te debuggen zijn. Om dit te voorkomen, kunt u overwegen de duidelijkheid van foutmeldingen te verbeteren en invoervalidatie toe te voegen om de hierboven genoemde zorgen weg te nemen.

update: Gedeeltelijk gefixeerd plegen 63b6a95 en plegen 7c0ade7. Het is alleen gevalideerd als de inputAmount waarde nul is en de volgorde van bewerkingen is verplaatst om vroegtijdig te mislukken bij het branden van LP-tokens om de gaskosten te verlagen. Verklaring van het 1inch-team voor dit probleem:

tokenFrom en tokenTo validatie is niet verplicht, omdat ze in de swap-methode worden vervangen door constanten

[L04] Constanten niet gedeclareerd of duidelijk benoemd

In het FixedRateSwap contract, de letterlijke waarden 998, 1000 en 1002, die worden gebruikt om de kosten in de _getRealAmountsForWithdrawImpl en _getVirtualAmountsForDepositImpl functies, hebben geen begeleidende uitleg of inline documentatie.

Bovendien heeft een gebrek aan inline-documentatie ook invloed op de dubbelzinnig benoemde constanten die worden gebruikt in berekeningen in de hele codebase.

Om de leesbaarheid van de code te verbeteren en refactoring te vergemakkelijken, kunt u overwegen een constante te definiëren voor elke "magische waarde" en ervoor te zorgen dat alle constanten duidelijke en voor zichzelf sprekende namen hebben. Overweeg voor complexe waarden om inline documentatie toe te voegen waarin wordt uitgelegd hoe ze zijn berekend of waarom ze zijn gekozen.

update: Vast in plegen 7091076.

[L05] Misleidende of onvolledige inline documentatie

In de hele codebase zijn enkele gevallen van misleidende en/of onvolledige inline-documentatie geïdentificeerd die moeten worden verholpen.

Hier volgen voorbeelden van onvolledige inline-documentatie:

Hieronder volgen voorbeelden van misleidende inline-documentatie:

  • De berekeningen van de privéfunctie _powerHelper vermeld niet expliciet dat het resultaat van de vermenigvuldiging wordt gedeeld door 1e18 om dubbele schaling van de waarde te voorkomen. Bovendien komt de exponent van de schaalfactor ook overeen met de macht die door de functie wordt gegeven, wat voor extra verwarring zou kunnen zorgen als deze niet wordt opgemerkt.

Duidelijke inline documentatie is van fundamenteel belang om de bedoelingen van de code te schetsen. Mismatches tussen de inline documentatie en de implementatie kunnen leiden tot ernstige misvattingen over hoe het systeem zich moet gedragen. Overweeg eventuele fouten te herstellen en waar nodig aanvullende documentatie toe te voegen om verwarring voor zowel ontwikkelaars, gebruikers als auditors te voorkomen.

update: Erkend, en zal niet repareren.

[L06] Geen toegankelijk dekkingsrapport

Hoewel de README filet verwijst naar een dekkingsrapport, is ontoegankelijk.

Bovendien zijn er geen instructies voor het uitvoeren van de testdekkingsscripts.

Overweeg om het dekkingsrapport toegankelijk te maken en expliciet te documenteren hoe de testdekkingsscripts moeten worden uitgevoerd.

update: Gedeeltelijk vast. Het dekkingsrapport is nu toegankelijk, maar de README bestand legt nog steeds niet uit hoe de scripts moeten worden uitgevoerd.

[L07] Potentieel onveilige niet-aangevinkte wiskunde

Gehele FixedRateSwap contract, er zijn veel toepassingen van unchecked wiskunde. De belangrijkste reden voor het gebruik: unchecked wiskunde is om overloop-/onderstroomcontroles te verwijderen in gevallen die ofwel afhankelijk zijn van dergelijk gedrag of waarvan bekend is dat ze niet onderlopen/overlopen. Dit heeft als voordeel dat er gas wordt bespaard, maar kan bij verkeerd gebruik leiden tot onverwachte resultaten en mogelijke kwetsbaarheden.

Gevallen van unchecked rekenkunde die potentieel kan onder-/overlopen werden geïdentificeerd. Bijvoorbeeld:

De noodzakelijke waarden die nodig zijn om deze berekeningen te overlopen, gecombineerd met redelijke validaties in de hele codebase, zorgen er vaak voor dat deze overlopen in de praktijk niet haalbaar zijn, maar ze zijn nog steeds theoretisch mogelijk. Overweeg om niet te gebruiken unchecked wiskunde behalve wanneer de mogelijkheid van overlopen kan zijn compleet uitgesloten, om onverwachte resultaten te voorkomen en het algehele aanvalsoppervlak van het protocol te verminderen.

update: Erkend, en zal niet repareren.

[L08] Onveilig expliciet casten van gehele getallen

De Swap evenement duurt een address en twee int256 parameters en het wordt uitgezonden in de swap0To1, swap1To0, swap0To1For en swap1To0For functies.

Tijdens de emissie worden de gehele waarden die aan de gebeurtenis worden doorgegeven, echter expliciet verwijderd van uint256 naar int256 waarden.

Hoewel het in de praktijk vandaag de dag waarschijnlijk niet problematisch zal zijn, kunnen ontwikkelingen in ecosystemen, zoals onbeperkte flitsleningen van stablecoin-activa, ertoe leiden dat dit ontwerp in de toekomst ongewenst gedrag vertoont. Op een gegeven groot genoeg uint256 waarde, expliciet gieten in een int256 type zou de waarde ervan afkappen. Als gevolg hiervan kunnen off-chain systemen, afhankelijk van de nauwkeurigheid van de gebeurtenisemissie, worden misleid.

Overweeg om de opnieuw te definiëren Swap evenement om direct mee om te gaan uint256 waarden zodat de functies die de gebeurtenis uitzenden de expliciete casts kunnen missen.

update: Vast in plegen 8436c6c. 1inch-team de OpenZeppelin's geïmporteerd SafeCast bibliotheek om de genoemde gevallen veilig te casten.

[L09] Het terugtrekkingsproces kan leiden tot vloerbedekking

Wanneer een aandeelhouder ofwel de withdraw of de withdrawFor functies, het contract berekent het bedrag van de activa dat ze recht hebben op een gegeven aantal aandelen. Die activa worden vervolgens overgedragen aan de opgegeven ontvanger.

Sindsdien echter if uitspraken worden gebruikt dan require verklaringen om te valideren of activa naar de ontvanger moeten worden verzonden, als de pool onevenwichtig is en het aantal aandelen klein is, kan het contract de te verzenden waarde voor een of beide activa onderdrukken.

Gezien het feit dat de betrokken waarden noodzakelijkerwijs vrij klein zouden zijn, en gezien het feit dat het protocol opnames niet volledig kan beperken waarbij één token-output in feite nul kan zijn, overweeg dan om dit potentiële afrondingsgedrag te documenteren, zodat gebruikers zich ervan bewust zijn bij het opnemen.

update: Erkend, en zal niet repareren.

Opmerkingen en aanvullende informatie

[N01] Verouderde projectafhankelijkheden

Tijdens de installatie van de afhankelijkheden van het project, NPM waarschuwt dat een van de geïnstalleerde pakketten, Highlight, "zal in de toekomst niet langer worden ondersteund of beveiligingsupdates ontvangen".

Hoewel het onwaarschijnlijk is dat dit pakket een veiligheidsrisico kan veroorzaken, kunt u overwegen de afhankelijkheid die dit pakket gebruikt te upgraden naar een onderhouden versie.

update: Vast in plegen 0a2b55d. De installatie vereist nu echter het gebruik van de -force vlag op huidige LTS-versies van node om te slagen.

[N02] Poolvergoedingen kunnen onevenwichtige deposito's stimuleren

Bij het storten in de FixedRateSwap contract, het _getVirtualAmountsForDeposit functie berekent de virtuele waarde van de aanbetaling. Virtuele bedragen zijn de oorspronkelijk geschaalde bedragen, na het in rekening brengen van de vergoeding volgens de huidige activaratio van de pool.

Als een gebruiker geld stort bij de huidige verhouding van activa binnen de pool dan worden er geen kosten in rekening gebracht. Anders worden er kosten in rekening gebracht op basis van het verschil tussen hun depositoratio en de huidige activaratio van de pool.

Dit ontwerp houdt in dat wanneer de verhouding van activa binnen de pool niet in evenwicht is, gebruikers zouden worden gestimuleerd om te storten met dezelfde onevenwichtige verhouding, in plaats van te storten met een verhouding die de pool evenwichtiger zou maken.

Om een ​​evenwichtiger pool aan te moedigen, kunt u overwegen deposito's die de pool in evenwicht houden te stimuleren in plaats van ze te bestraffen. Als alternatief, als dit niet haalbaar is, overweeg dan om in de documentatie van het project uit te leggen waarom.

update: Erkend, en zal niet repareren.

[N03] Ongedocumenteerde impliciete goedkeuringsvereisten

De FixedRateSwap contract veronderstelt impliciet dat het een passende vergoeding heeft gekregen voordat het wordt uitgevoerd swaps en deposito's die noodzakelijkerwijs tokens overdragen.

Ten gunste van explicietheid en om de algehele duidelijkheid van de codebase te verbeteren, kunt u overwegen om alle goedkeuringsvereisten te documenteren in de inline documentatie van de relevante functies.

update: Erkend, en zal niet repareren.

[N04] Verwarrende impliciete validatie van outputAmount

De getReturn functie is voorzien van drie parameters, namelijk een token om van te wisselen (tokenFrom), een token om naar te ruilen (tokenTo), en een invoerbedrag (inputAmount waarde van de tokenFrom Bedrijfsmiddel). de overeenkomstige outputAmount waarde (resulterend bedrag voor de tokenTo actief) is dan berekend en geretourneerd.

Vóór de berekening, de functie: vereist dat de inputAmount waarde is kleiner dan of gelijk aan het tokensaldo van de pool van de tokenTo Bedrijfsmiddel. echter, de outputAmount waarde, misschien wel de meer intuïtieve waarde om te vergelijken met de tokenTo activabalans, wordt nooit expliciet gecontroleerd op dezelfde voorwaarde.

Eigenlijk, de wiskunde die wordt gebruikt om te berekenen outputAmount waarde zorgt ervoor dat het strikt kleiner is dan of gelijk is aan de inputAmount waarde.

De opzet van dit gedrag is echter onduidelijk, dwz het is niet duidelijk of dit ontwerp alleen bedoeld is om sneller te falen tijdens de uitvoering om de gaskosten te verlagen of niet. Overweeg om de redenering voor de exacte gebruikte controle expliciet te documenteren. Overweeg bovendien om te valideren of het saldo van de to actief is groter dan de outputAmount waarde in tegenstelling tot de inputAmount waarde.

update: Vast in plegen 10f4d9c.

[N05] Inconsistentie naamgeving

De FixedRateSwap contract stelt een expliciete paar penningen waarmee de ruil-, opname- en stortingsoperaties bedoeld zijn. Door de hele codebase zijn deze tokens gelabeld met een index van ofwel 0 or 1, als in token0 en token1. Functies die beschrijvend worden genoemd om details over het gebruik over te brengen, maken ook gebruik van die token-indices, zoals de swap0To1 functie.

Echter, voor de withdrawForWithRatio functie, de parameter die de te ontvangen verhouding definieert token0 tegen token1 is genaamd firstTokenShare wat voor verwarring zou kunnen zorgen dat het verwijst naar de token1 aanwinst en niet de token0 troef.

Overweeg om de naamgevingsconventies consistent te houden in de hele codebase om de algehele leesbaarheid te verbeteren en mogelijke verwarring te verminderen.

update: Vast in plegen 57ad4cd.

[N06] Berichten voor terugzetten zijn inconsistent opgemaakt

De require uitspraken in de constructor van de FixedRateSwap contract zijn anders opgemaakt dan alle andere require verklaringen in het contract.

Omdat inconsequent opgemaakte terugzetberichten voor onnodige verwarring kunnen zorgen, moet u ervoor zorgen dat alle require verklaringen hebben teruggekeerde berichten die consistent zijn opgemaakt, nauwkeurig, informatief en gebruiksvriendelijk zijn.

update: Vast in plegen 0aa4e9d.

[N07] Inconsistent gebruik van benoemde retourvariabelen

Er is een inconsistent gebruik van benoemde retourvariabelen in de FixedRateSwap contract.

In het bijzonder, terwijl de meeste functies benoemde variabelen retourneren, is de decimals, _getVirtualAmountsForDepositImpl, _getRealAmountsForWithdrawImpl en _checkVirtualAmountsFormula functies retourneren expliciete waarden.

Overweeg een consistente benadering te hanteren voor het retourneren van waarden in de hele codebase door alle benoemde retourvariabelen te verwijderen, ze expliciet als lokale variabelen te declareren en waar nodig de benodigde retourinstructies toe te voegen. Dit zou zowel de explicietheid als de leesbaarheid van de code verbeteren, en het kan ook helpen bij het verminderen van regressies tijdens toekomstige code-refactoren.

update: Erkend, en zal niet repareren.

[N08] Gasoptimalisaties

Binnen FixedRateSwap contract zijn er mogelijkheden voor enkele eenvoudige besparingen op het gasverbruik. Bijvoorbeeld:

  • De _getVirtualAmountsForDeposit private functie wordt altijd maar vanaf één plaats in de codebase aangeroepen, en dat is de depositFor functie. Binnen de depositFor functie zijn er oproepen naar token0.balanceOf(address(this)) en token1.balanceOf(address(this)). Diezelfde oproepen worden echter bovenaan de _getVirtualAmountsForDeposit functie. De laatste functie kan eenvoudig de vereiste waarden worden doorgegeven, in plaats van de saldi twee keer per depositFor noemen.
  • Binnen _getRealAmountsForWithdrawImpl private functie, de secondTokenShare variabele is gedefinieerd als _ONE - firstTokenShare. Op de volgende regel, wordt exact dezelfde aftrekking opnieuw uitgevoerd wanneer het gewoon de zou kunnen gebruiken secondTokenShare variabel.

Om de gaskosten te verlagen en de codebase verder te vereenvoudigen, kunt u overwegen de hierboven genoemde gevallen aan te pakken.

update: Vast in plegen 34974ee.

[N09] Onjuiste functiezichtbaarheid

De withdrawWithRatio functie wordt niet intern aangeroepen door een van de functies in de FixedRateSwap contract. Overweeg de zichtbaarheid in te stellen op: external in plaats van public.

update: Vast in plegen cb852f5.

[N10] Vertrouwen op matching decimals kan problematisch zijn

Het protocol vereist impliciet dat beide tokens in een pool de decimals methode om de zwembadbouw succesvol te laten zijn. In werkelijkheid is deze methode bijna alomtegenwoordig, maar het is technisch gezien een optioneel onderdeel van de ERC20-specificatie waarin expliciet wordt vermeld dat contracten "NIET verwachten dat deze waarden aanwezig zijn". Momenteel zijn alle tokens die de optionele decimals methode zal niet bruikbaar zijn binnen het protocol.

Misschien problematischer, als onderdeel van deze oproepen tot token decimals, vereist het protocol verder dat beide tokens identieke waarden retourneren.

De ruimte voor stabiele munttokens is in dit opzicht echter niet homogeen. Het bestaat uit veel tokens die een verscheidenheid aan verschillende waarden retourneren voor: decimals. Bijvoorbeeld, terwijl USDT en USDC rapporteer 6 decimalen, DAI rapporteert 18 decimalen.

Als dit opzettelijke beperkingen van het protocol zijn, overweeg dan om ze expliciet aan te raken en korte rechtvaardigingen te geven via de inline-documentatie. Overweeg ook om tijdens de bouw betere foutmeldingen te geven voor tokens die de decimals methode. U kunt ook overwegen om het protocol robuuster te maken, zodat het tokens aankan die de niet ondersteunen decimals methode of paren die ongelijksoortig rapporteren decimals binnen hetzelfde zwembad.

update: Vast in plegen b49d808. Inline documentatie is toegevoegd.

[N11] Typografische fout

We hebben de volgende typografische fout in de codebase vastgesteld:

  • Het terugzetbericht aan lijn 92 of FixedRateSwap.sol begint zonder hoofdletter, waardoor het inconsistent is met de rest van de code.

Om de algehele consistentie en leesbaarheid van de codebase te verbeteren, kunt u overwegen deze en andere typografische fouten in de codebase te corrigeren.

update: Vast in plegen a92d16a.

[N12] Onnodig virtual functie

De FixedRateSwap contract erft van OpenZeppelin's ERC20 contract maar het is overschrijft zijn ERC20.decimals functie. Dit is nodig omdat de FixedRateSwap's liquiditeitspool-token, afhankelijk van de decimals van de activa waaruit de pool bestaat, is noodzakelijkerwijs dynamisch.

Hoewel deze allesoverheersende implementatie van de functie definitief zou moeten zijn, is het: gedefinieerd met de virtual trefwoord, wat aangeeft dat het niet per se een definitieve implementatie is en het mogelijk maakt om het opnieuw te negeren.

Om verwarring te voorkomen en de bedoeling te verduidelijken, kunt u overwegen de virtual trefwoord of het documenteren van de redenen om het te behouden.

update: Vast in plegen 8bce5ec.

Conclusies

Er zijn geen problemen met een hoge ernst gevonden. Er zijn enkele wijzigingen voorgesteld om de beste praktijken te volgen en het potentiële aanvalsoppervlak te verkleinen.

Tijdstempel:

Meer van Zeppelin openen