Coder Social home page Coder Social logo

dss's Introduction

Multi Collateral Dai

Build Status

This repository contains the core smart contract code for Multi Collateral Dai. This is a high level description of the system, assuming familiarity with the basic economic mechanics as described in the whitepaper.

Additional Documentation

dss is also documented in the wiki and in DEVELOPING.md

Design Considerations

  • Token agnostic

    • system doesn't care about the implementation of external tokens
    • can operate entirely independently of other systems, provided an authority assigns initial collateral to users in the system and provides price data.
  • Verifiable

    • designed from the bottom up to be amenable to formal verification
    • the core cdp and balance database makes no external calls and contains no precision loss (i.e. no division)
  • Modular

    • multi contract core system is made to be very adaptable to changing requirements.
    • allows for implementations of e.g. auctions, liquidation, CDP risk conditions, to be altered on a live system.
    • allows for the addition of novel collateral types (e.g. whitelisting)

Collateral, Adapters and Wrappers

Collateral is the foundation of Dai and Dai creation is not possible without it. There are many potential candidates for collateral, whether native ether, ERC20 tokens, other fungible token standards like ERC777, non-fungible tokens, or any number of other financial instruments.

Token wrappers are one solution to the need to standardise collateral behaviour in Dai. Inconsistent decimals and transfer semantics are reasons for wrapping. For example, the WETH token is an ERC20 wrapper around native ether.

In MCD, we abstract all of these different token behaviours away behind Adapters.

Adapters manipulate a single core system function: slip, which modifies user collateral balances.

Adapters should be very small and well defined contracts. Adapters are very powerful and should be carefully vetted by MKR holders. Some examples are given in join.sol. Note that the adapter is the only connection between a given collateral type and the concrete on-chain token that it represents.

There can be a multitude of adapters for each collateral type, for different requirements. For example, ETH collateral could have an adapter for native ether and also for WETH.

The Dai Token

The fundamental state of a Dai balance is given by the balance in the core (vat.dai, sometimes referred to as D).

Given this, there are a number of ways to implement the Dai that is used outside of the system, with different trade offs.

Fundamentally, "Dai" is any token that is directly fungible with the core.

In the Kovan deployment, "Dai" is represented by an ERC20 DSToken. After interacting with CDPs and auctions, users must exit from the system to gain a balance of this token, which can then be used in Oasis etc.

It is possible to have multiple fungible Dai tokens, allowing for the adoption of new token standards. This needs careful consideration from a UX perspective, with the notion of a canonical token address becoming increasingly restrictive. In the future, cross-chain communication and scalable sidechains will likely lead to a proliferation of multiple Dai tokens. Users of the core could exit into a Plasma sidechain, an Ethereum shard, or a different blockchain entirely via e.g. the Cosmos Hub.

Price Feeds

Price feeds are a crucial part of the Dai system. The code here assumes that there are working price feeds and that their values are being pushed to the contracts.

Specifically, the price that is required is the highest acceptable quantity of CDP Dai debt per unit of collateral.

Liquidation and Auctions

An important difference between SCD and MCD is the switch from fixed price sell offs to auctions as the means of liquidating collateral.

The auctions implemented here are simple and expect liquidations to occur in fixed size lots (say 10,000 ETH).

Settlement

Another important difference between SCD and MCD is in the handling of System Debt. System Debt is debt that has been taken from risky CDPs. In SCD this is covered by diluting the collateral pool via the PETH mechanism. In MCD this is covered by dilution of an external token, namely MKR.

As in collateral liquidation, this dilution occurs by an auction (flop), using a fixed-size lot.

In order to reduce the collateral intensity of large CDP liquidations, MKR dilution is delayed by a configurable period (e.g 1 week).

Similarly, System Surplus is handled by an auction (flap), which sells off Dai surplus in return for the highest bidder in MKR.

Authentication

The contracts here use a very simple multi-owner authentication system, where a contract totally trusts multiple other contracts to call its functions and configure it.

It is expected that modification of this state will be via an interface that is used by the Governance layer.

dss's People

Contributors

asymmetric avatar brianmcmichael avatar d-xo avatar daimesava avatar desaperados avatar dizzy avatar gbalabasquer avatar godsflaw avatar hexonaut avatar iamchrissmith avatar julienmartinlevrai avatar k06a avatar kentonprescott avatar kmbarry1 avatar livnev avatar mhhf avatar mrchico avatar nanexcool avatar rainbreak avatar xmxanuel avatar xwvvvvwx avatar yaronvel avatar zdumitru avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

dss's Issues

lodash pollution

Detailed paths
Introduced through: @makerdao/[email protected]lodash@4.17.13
Fix: Upgrade to [email protected]
Overview
lodash is a modern JavaScript utility library delivering modularity, performance, & extras.

Affected versions of this package are vulnerable to Prototype Pollution in zipObjectDeep due to an incomplete fix for CVE-2020-8203

Return early if `now == rho`.

dss/src/pot.sol

Line 138 in effdda3

require(now >= rho);

This function is a no-op if rho == now. Rather than wasting a bunch of effort (gas) executing this function in that case, it should just return early. While the caller can do if (pot.rho != now) pot.drip(), it is much more expensive for them to do this than it is for this method to do that.

Calling wipeandfree function from front-end using web3.js

In order interact with SaiProxy we need to execute that script through DSProxy of my account, right?
The function to be inkoved is execute(address target, bytes data) right?
When I did a transaction in kovan network from makerdao-cdp portal the encoded arugments of the wipeandfree are not matching with the number of arguments of execute() function.
Can anyone explain clearly what am I missing here and how to convert encoded functions arguments such as uint,address to bytes type since that is the datatype require in execute() functiona arguments.

Expose liquidation penalty as a public function

I would kindly ask that the cat would expose the tab calculation as a public function.
A similar function exists in compound as an API (in their smart contract).
A PR was submitted #133

The motivation is that we are working on an opt-in liquidation system that needs to read the liquidation penalty value from makerdao’s cat.
We plan to read the cat address from the end contract, but changes like in this version, where the chop resolution changed from rad to wad will break our system (in the future, it is still not deployed).
Hence, a function that for a given art returns the tab, would really help.

Check bid still exists before calling tick

If an auction has been dealt and tick is called after but in the same block, there's some residue left in storage (end in Flipper and Flapper, end and lot in Flopper ).

Adding a require(bids[id].guy != address(0), "Fl*pper/auction-not-active"); to the top of tick fixes this.

Not an issue now, but we want to be good citizens and not bloat state.

Missing dai in dai stable coin adress

I send DAI TOKEN FRON MY A/C TO UNISWAP DAI 126.44 ADRESS 0x6B175474E89094C44Da98b954EedeAC495271d0F on 27may2021 but my money not show in uniswap but my money show in etherscan Trans hash- 0x93ee3b5be28b7f436dfed66c6146eb62e5aa01e9ce8164ea0532689d931bbf90 pls detect my money for return on these adress- 0xcD77Cd70CbCEE93e9681126eEfa5e34eB9130DdC pls contact me email id [email protected] solve the problem pls inform me next and solve my problem my money is block not reply from uniswap pls provide next prosess I attach some screen shot for I send DAI trnsa hash and Show in Etherscan

Fix compiler warnings.

dss/src/vat.sol

Lines 93 to 97 in 17be7db

function add(uint x, int y) internal pure returns (uint z) {
z = x + uint(y);
require(y >= 0 || z <= x);
require(y <= 0 || z >= x);
}

dss/src/vat.sol

Lines 108 to 110 in 17be7db

function add(uint x, uint y) internal pure returns (uint z) {
require((z = x + y) >= x);
}

vat.sol:93:5: Warning: Variable is shadowed in inline assembly by an instruction of the same name
    function add(uint x, int y) internal pure returns (uint z) {
    ^ (Relevant source part starts here and spans across multiple lines).

DAI seem to be stuck in contract

Hi Makerdao team.

Thanks for all your great work. It's the first time I encounter an error which I can't solve on my own in this context:

It is about the following transactions

https://etherscan.io/tx/0xfd51cf46707de823824dc6aec3f9fdc887eff2da53fbd8a49bc2b99d7fee84ae
https://etherscan.io/tx/0x0fb05d52c4163ce66f84c261481948b768c068488a9620c92e4c9f7befb39e3b
It seems that the 548.5080525794122 DAI got stuck somewhere :) Can't find them (not in sender's nor in receiver's wallet. It would be great if you could help me out.

Kind regards
Michael

Hey old investor and holder with smart contracts helping and old project with security

You have multiple vulns in your code and python and unupdated os. Also in smart contracts,

makerdao/market-maker-keeper
83
C
261
H
291
M
468
L
83
C
260
H
286
M
468
L

Sol: Hardcoded value is used as a cipher key (in ethereumjs-util.privateToAddress). Generate the value with a cryptographically strong random number generator and do not hardcode it in source code. get back to me so i help you and fix this.

best regards Philip cyptocurrencies sweden

ERC20 Approval event conformance

I found that OZ ERC20 implementation differs with your DAI implementation.

ERC20 Token Standard tells:

Approval
MUST trigger on any successful call to approve(address _spender, uint256 _value).

For me it seems OZ implementation should be fixed to conform, or maybe they have some reasons to follow this behavior.

Change the order of operations in pot

dss/src/pot.sol

Lines 149 to 151 in effdda3

pie[msg.sender] = add(pie[msg.sender], wad);
Pie = add(Pie, wad);
vat.move(msg.sender, address(this), mul(chi, wad));

While I don't think there is a specific reentrancy attack here, it is not recommended to update the balance first and then move the underlying asset. Instead, you should move the underlying asset first, then after that is successful update the balance.

Add ERC20-compatible Escape Hatch to Gem Join Contracts

It's a common pattern to include an ERC20 token extraction mechanism in rapidly developed contracts in case of accidentally trapped funds. I think it could be useful to upgrade all the gem join contracts to be compliant with part of the ERC20 standard - specifically transfer, transferFrom and balanceOf. This would basically provide an ERC20-compliant escape hatch for accidentally stuck collateral to be extracted from Maker.

The functions would look something like this (pseudocode):

function balanceOf (account) {
	// Return the ink that can be freed from vat.frob (after repaying with the available vat.dai) and any vat.gem available
}

function transfer (address recipient, uint amount) {
	if (vat.gem[ilk][msg.sender] < amount) {
		// Pull the ink out of the cdp and repay as much dai as available
		vat.frob(ilk, msg.sender, msg.sender, msg.sender, -int(sub(amount, vat.gem[ilk][msg.sender])), -int(vat.dai[msg.sender]));
	}

	exit(recipient, amount);
}

function transferFrom (address sender, address recipient, uint amount) {
	require(sender == msg.sender)

	transfer(recipient, amount);
}

This would allow anyone to completely unwind a CDP with only the ERC20 transfer semantics as any account is able to repay debt. So for example to unwind my locked CDP, I can do: Flash Loan (Dai), Repay all CDP Debt, gemJoin.transfer(myAccount, gemJoin.getBalance()), Trade Gem for Dai, Repay Flash Dai Loan.

The only downside I can see to this is a slight increase in code complexity, but we get the added benefit of lower liquidations from trapped funds.

Ensure `Cat.bite` will not create dusty/too-small CDPs

Cat.bite takes lump-sized chunks out of underwater CDPs for auction. Depending on the math, this process may end up with a tiny about of debt+collateral not liquidated for auction. If the collateral amount is too small (in terms of market value relative to gas costs for liquidation and bidding), it will be economically irrational for Keepers to liquidate and bid on it. While not an immediate issue, over long times, especially with many such positions in existence, the system could unfortunately accrue large amounts of unbacked debt.

While it is possible to combat this through proactively finding an eliminating such positions (altruistic liquidation, e.g. by MKR holders concerned about the long-term health of the system), it would be much nicer if the system just avoided creating such positions in the first place.

lack of tests for `rpow` (?)

rpow is implemented in jug.sol and drip.sol. The function is complex enough to deserve some dedicated tests but I do not find any in the dss repository.

I am guessing the function was first implemented elsewhere and was tested there, and would appreciate if I could get a reference for it.
I am working on a project that integrates with the dss, and needs to duplicate the rpow logic. Hence, a reference for the tests would help me test my code.

Missing Dai in DAI stable coin

I send DAI TOKEN FRON MY A/C TO UNISWAP DAI 126.44 ADRESS 0x6B175474E89094C44Da98b954EedeAC495271d0F on 27may2021 but my money not show in uniswap but my money show in etherscan Trans hash- 0x93ee3b5be28b7f436dfed66c6146eb62e5aa01e9ce8164ea0532689d931bbf90 pls detect my money for return on these adress- 0xcD77Cd70CbCEE93e9681126eEfa5e34eB9130DdC pls contact me email id [email protected] solve the problem pls inform me next and solve my problem my money is block not reply from uniswap pls provide next prosess I attach some screen shot for I send DAI trnsa hash and Show in Etherscan 

Debt Auctions Can Be Blocked

Fixing Debt Auction Blocking

Background

If surplus collected from a debt auction is canceled with free unbacked debt (woe) using Vow.heal instead of with on-auction debt (Ash) via kiss, future debt auctions (flops) can be prevented from occurring (see these slides for an in-depth explanation; discussion occurred in this governance call). While governance can take action to unblock these debt auctions, it would be much better if the system either prevented this situation entirely or allowed it to be resolved without a governance vote.

Primary Fix Options

While far from exhaustive, the options presented here cover what are probably the simplest and least disruptive fix options, and are thus the main ones under debate.

Instant Access Module

When an amount zed of debt becomes "stuck" as Ash, governance can unblock the stuck debt by converting it into woe via calling first Vat.suck(address(Vow), address(Vow), zed) and then Vow.kiss(zed). An appropriately permissioned (relied upon by the Vat) instant access module could enable this operation to be be done by anyone at any time, without requiring a governance vote.

The obvious attractive feature of this solution is that it requires no replacements of any existing core contracts. It is complicated, however, by the need to avoid "unsticking" auction debt that has not yet received a bid (otherwise extra, unnecessary debt auctions could occur). The simplest solution to this issue is to have the module keep track of how much debt is up for auction but as-of-yet unbid upon. This can be done without loops in the contract itself, for example as follows:

// Define here mock interfaces for Vat, Vow, and Flopper.

contract FlopUnblocker {
    // Define here obviously needed things like a constructor,
    // safe math functions, etc.

    Vat public vat;
    Vow public vow;
    Flopper public flop;

    mapping (uint => uint) public known;
    uint public knownAuctionCount;
    mapping (uint => uint) public ashes;
    uint public trueAsh;

    function unblock() public {
        require(knownAuctionCount == flop.kicks());

        // kiss away as much debt as possible
        uint joy = vat.dai(address(vow));
        require(joy < vow.Ash());  // optional: could instead kiss away Ash
        vow.kiss(joy);

        // unblock stuck auctions
        uint zed = sub(vow.Ash(), trueAsh);
        vat.suck(address(vow), address(vow), zed);
        vow.kiss(zed);
    }

    function update(uint id) public {
        require (id <= flop.kicks());

        if (known[id] == 0) {
            known[id] = 1;
            ++knownAuctionCount;
        }

        (uint bid,, address guy, uint48 tic,) = flop.bids(id);

        if (guy != address(0) && tic == 0) {
            // auction is active and unbid-upon
            if (ashes[id] == 0) {
                trueAsh = add(trueAsh, bid);
                ashes[id] = bid;
            }
        } else {
            // auction has received a bid
            trueAsh = sub(trueAsh, ashes[id]);
            ashes[id] = 0;
        }
    }
}

Adding this module allows easy corrective action to be taken at any time, but it also increases the complexity of the system and carries a certain amount of technical risk (primarily since it requires authorization against the Vat and calls suck). It also enshrines managing "stuck" debt as a permanent cognitive overhead for ecosystem participants (although likely in the long-term automation logic, e.g. in Keeper bots, would reduce this overhead to near-zero).

Call kiss Upon First dent

This constitutes adding the following logic just after the call to Vat.move in dent:

if (bids[id].tic == 0) {
    Vow vow = Vow(bids[id].guy);  // first guy = gal = Vow
    uint rad = vow.Ash() >= bid ? bid : vow.Ash();
    vow.kiss(rad);
}

The advantage of this solution is its simplicity and obvious correctness.

The major downside is the need to replace the Flopper contract, which can break other deployed smart contracts and require updates to UIs, Keepers, and other off-chain software.

Minor downsides include slightly increased gas costs for dent, and a slightly higher gas cost for the first dent as compared to subsequent dents in a given auction.

There is also an architectural inelegance—the Vow is dependent on the Flopper behaving in a certain way to ensure correct debt handling.

heal Reverts If Ash Is Non-zero

While this trivially prevents flops from being blocked by requiring Ash to be eliminated via kiss before healing can take place, it requires replacing the Vow, a much more risky modification to the system than either replacing the Flopper or adding an instant access module. It also has some probability of breaking contracts or Keepers that do not prioritize kissing debt over healing debt (not to mention upgrades and updates of contracts and bots that depend on the Vow's address). It is, however, architecturally pure—the Vow can behave correctly regardless of how other contracts or users call its unauthorized functions.

Other Fix Options

Re-Architect Debt Auction Interactions

E.g., have the Flopper receive the dai bids, only transfer to the Vow on deal, and make kiss only callable by the Flopper itself (which it would do during deal). This is a structurally pleasing option in that it makes flop mechanics more similar to those of flap, but replacing both the Vow and the Flopper while invalidating a lot of Keeper code is probably not prudent since the system is already live. It also couples the Vow and Flopper more tightly than is necessarily wise. There's more design space here, but there's not a lot of point in exploring it given that the system is already deployed and thus minimizing change is preferable.

add automated toolings to rename methods/variables, for instructional aids

i know this bike-shed had been fought over many times in past ages... but i've built a little script convert these names to more standard names.

i've made a little script to do it, so even if code updates in the future it'd be easy to convert quickly with a github hook.

https://github.com/hayeah/dss/tree/rename#renaming

To aid new developers, perhaps consider two options:

  1. maintain a branch for the non-quirky naming scheme, or
  2. include the toolings as part of DEVELOPING.md

liquidation 2.0: add bark's tip to tab (user debt)

In liquidation 1.2, bite cost over $50, and during network congestion this could be tripled.
Currently the tip is paid (to my understanding) by the system.
This will force high dust, where the liquidation penalty exceeds the proposed tip.

By making the tip part of the liquidation penalty, lower dust levels could be supported.

Consider removing 1.3 of the purple paper for readability

I'm not sure if there is a tracking issue for this already, but I would like to start a formal request not to use the language outlined in section 1.3 of the MakerDAO purple implementation paper.

Here are my arguments:

  • The code is unreadable without a memorized glossary or frequently referring to the written glossary.
  • Given this code is hard to read, it will be even harder to integrate with. This slows adoption of MCD in other contracts.
  • The benefit of using "concise names" have a one time benefit of reasoning about the system in a mathematical paper, but burden smart contract readers for the rest of time.

Addressing the motivations outlined in the purple paper:

We sidestep terminological debates; for example, whether to say »rate of target price change« or »target rate«.

This type of bikeshedding doesn't seem more of a concern than deciding whether or not to call something bump, hump, or sump.

With decoupled financial and technical vocabularies, we can more flexibly improve one without affecting the other.

That can still be done with proper variable names like liquidationPenalty rather than chop. In fact, this information is still present in the code so why not use it as the variable/property names?

The ability to discuss the system formally, with the financial interpretation partly suspended, has suggested insights that would have been harder to think of inside the normal language.

I can appreciate this, but perhaps a better solution is the inverse glossary where the formal discussion of properties can be translated into real words.

The precise and distinctive language makes the structure and logic of the implementation more apparent and easier to formalize.

I argue that this pattern makes the structure and logic less apparent. Again, it probably is easier to formalize, but harder to read from the code perspective. The formalization happens once while the code is read many times in the future.

Concise names make the code less verbose and the concepts easier to handle on paper, whiteboard, etc.

Less verbose isn't a good thing when you lose information/context in your code.

Please consider refactoring the code to be human readable.

Wiki glossary: ambiguous usage of `collateral` notion in `flux` and `slip`

From the wiki glossary:
flux: transfer collateral between users
slip: modify a user's collateral balance
ink: collateral balance

The glossary does not make a distinct definition to user's collateral (gem) as opposed to urn's collateral (ink).
Hence, it might be better understood if flux description will be "transfer gem between users", or alternatively "transfer collateral token between users".
And similarly for slip.

This will make it easier to understand that these operations do not change the sate of a CDP/urn.

Maximum acceptable price

During testing of liquidation auction bidding (using clipper contract), I got several Clipper/too-expensive errors. According to the name of the variable and the comment, max argument of the take function is Maximum acceptable price (DAI / collateral) [ray], but later in the code it is checked to be smaller than the price:

require(max >= price, "Clipper/too-expensive");

The Problem

To my understanding, maximum acceptable price argument suppose to prevent bidding on an auction that is too expensive, right? So, for example, if maximum acceptable price is 10 and auction price is 20, it should revert the transaction. But currently it's the opposite: it prevents the transactions which are cheaper then expected (which should be totally fine for the end user). Moreover it is even exclusive of the current auction price, so sending current price as maximum acceptable price will also revert the transaction. Or maybe I misunderstood the meaning of this argument?

The Solution

To change the validation to be:

require(price > max, "Clipper/too-expensive");

Return chi from pot.drip()

Related to #87 (potentially incompatible with)

If you already have the pot's chi variable loaded from storage, it is worthwhile to return that value. Many (most?) external callers are likely to need that value immediately after calling pot.drip() (and will need to get that value in a subsequent call). Returning the data already loaded from storage would cost very little gas, a secondary call will cost ~2,000 gas

elliticp (high) cryptographfic

Detailed paths
Introduced through: @makerdao/[email protected][email protected]elliptic@6.3.3
Fix: Upgrade to [email protected]
Overview
elliptic is a Fast elliptic-curve cryptography in a plain javascript implementation.

Affected versions of this package are vulnerable to Cryptographic Issues. Elliptic allows ECDSA signature malleability via variations in encoding, leading \0 bytes, or integer overflows. This could conceivably have a security-relevant impact if an application relied on a single canonical signature.

Oasis does not work when using MetaMask with local full node

I am using the Oasis app at https://oasis.app/dashboard which I found on https://makerdao.com.

My issue is that when I connect to Oasis I get this error:

Browser ethereum provider and URL network param do not match!"

I am using MetaMask, connected to my own local full node running on Localhost 8545. When I switch MetaMask from "Localhost" network to "Mainnet", "Trust Infura" mode, then Oasis will work as expected. Something about the way Oasis is connecting to MetaMask doesn't work when I use Oasis and MetaMask connected to my own local full node instead of Infura, although it should work and there are other Ethereum apps I use that do work with my full node.

Sorry if this is the wrong place to post this issue. Even though the Oasis FAQ says the app is open source I could not find a repo for the Oasis app anywhere. This seemed like the next best place.

Is Oasis Secure?
Security is our top priority. We stringently follow the best security practices, and regularly conduct smart contract and code audits. In addition, Oasis code is open-source, giving everyone in the community the ability to pressure test and audit the core technology.

Source: https://oasis.app/support

Better logging

The note modifier often leaves something to be desired--for example, sometimes the most interesting and/or useful information to log is an updated storage value, not the function arguments. Other times, an event might be more user-friendly if certain logged values were not indexed (or vice-versa)...note has no way to specify which argument values should or should not be logged. Finally, note logs a fixed number of arguments and a fixed amount of calldata, but obviously this one-size-fits-all solution will generally be suboptimal.

An example: it would be nice if the event for Pot.drip() logged the latest chi value; c.f. the Tweet below.
https://twitter.com/LefterisJP/status/1230168954590695425

More generally, it would be nice to replace (or augment) note with custom events.

Add require messages to all require statements.

Trying to integrate with Maker is proving to be quite the chore because many of the require calls contain no message, which means when a transaction fails it is for an opaque reason. If each of the require statements contained a unique message (even if it was just file/line number) then it would be much easier to debug failures.

Jug.base does not conform to standard rate calculation

The base variable in jug.sol does not conform to the standard values we use for duty.

Currently we calculate duty values at the command line using

# (example 8%)
$ bc -l <<< 'scale=27; e( l(1.08)/(60 * 60 * 24 * 365) )'
> 1.000000002440418608258400030

where the result 1.000000002440418608258400030 is stored as a ray (example: 1000000002440418608258400030). This calculation works for both the individual collateral duty values as well as the dsr rate. This rate calculation will not apply to the base value in jug as the system is configured.

Currently, the system base value is 0, which works because base is added to the ilk.duty in drip()#104. Replacing this with a base rate value based on the duty calculation (ex. 1000000002440418608258400030 causes a revert in testing. Due to this rate calculation in drip, the base rate instead needs to be a smaller number, with a full RAY value subtracted, like 2440418608258400030, due to the addition operation. Even then, it's not as simple to calculate the base due to the compounding factors. Adding these two values will not produce an accurate compounded rate, for example:

// 8% duty calculation
eight_pct_duty                 = 1000000002440418608258400030

// example 1% base + 7% duty
one_pct_duty                   = 1000000000315522921573372069
seven_pct_duty                 = 1000000002145441671308778766
one_pct_duty + seven_pct_duty == 2000000002460964592882151000

// results are not equivalent
eight_pct_duty != (one_pct_duty + seven_pct_duty) - 10**27

The system works as expected with a base of 0, but I'm not sure the rates will operate as expected with a different base value. Any modification to base will need a new calculation that accounts for the drip() function not subtracting a RAY value from the added rates, and also accounts for the discrepancy in adding the base + duty compounded values.

Rename Variable, Function, and Contract names Sensibly

The naming conventions in the DSS contracts lack common sense.

The purpose of names are to describe the behavior of the function or variable. On a more local scale more general names such as "item", "vat", etc. are fine. But the high level naming of DSS are absolutely absurd.

Without docs, this code would be very difficult to decipher

I propose a complete renaming of the contracts andfunctions that actually reflect their function, rather than random "cat" and "dog" contracts

Is this a bug in dsr module ?

This document is mismatch my demo:

In the code, DSR is continuously compounding at a growth rate of x% per second.
For example, when the DSR is set to 2%, it will be accumulating at 1.0000000006279371924910298109948‬% per second.
Practically, users experience an annual compounding of the displayed rate.
For example, if the DSR was 2%, assuming the user put in 100 Dai, at the end of the first year they would have 102.00, and at the end of the second year they would have 104.04.

https://community-development.makerdao.com/makerdao-mcd-faqs/faqs/dsr#do-dsr-accruals-compound-or-are-they-only-earned-on-the-principal

This is my demo

/// pot.sol -- Dai Savings Rate

// Copyright (C) 2018 Rain <[email protected]>
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program.  If not, see <https://www.gnu.org/licenses/>.

pragma solidity >=0.5.12;

/*
   "Savings Dai" is obtained when Dai is deposited into
   this contract. Each "Savings Dai" accrues Dai interest
   at the "Dai Savings Rate".
   This contract does not implement a user tradeable token
   and is intended to be used with adapters.
         --- `save` your `dai` in the `pot` ---
   - `dsr`: the Dai Savings Rate
   - `pie`: user balance of Savings Dai
   - `join`: start saving some dai
   - `exit`: remove some dai
   - `drip`: perform rate collection
*/

// interface VatLike {
//     function move(address,address,uint256) external;
//     function suck(address,address,uint256) external;
// }

contract Pot {
    // --- Auth ---
    mapping (address => uint) public wards;
    function rely(address guy) external auth { wards[guy] = 1; }
    function deny(address guy) external auth { wards[guy] = 0; }
    modifier auth {
        require(wards[msg.sender] == 1, "Pot/not-authorized");
        _;
    }

    // --- Data ---
    mapping (address => uint256) public pie;  // Normalised Savings Dai [wad]

    uint256 public Pie;   // Total Normalised Savings Dai  [wad]
    uint256 public dsr;   // The Dai Savings Rate          [ray]
    uint256 public chi;   // The Rate Accumulator          [ray]

    // VatLike public vat;   // CDP Engine
    // address public vow;   // Debt Engine
    uint256 public rho;   // Time of last drip     [unix epoch time]

    uint256 public live;  // Active Flag

    mapping(address=>int256) public gains;

    // --- Init ---
    constructor(/*address vat_*/) public {
        wards[msg.sender] = 1;
        // vat = VatLike(vat_);
        dsr = ONE;
        chi = ONE;
        rho = now;
        live = 1;
    }

    // --- Math ---
    uint256 constant ONE = 10 ** 27;
    function rpow(uint x, uint n, uint base) internal pure returns (uint z) {
        assembly {
            switch x case 0 {switch n case 0 {z := base} default {z := 0}}
            default {
                switch mod(n, 2) case 0 { z := base } default { z := x }
                let half := div(base, 2)  // for rounding.
                for { n := div(n, 2) } n { n := div(n,2) } {
                    let xx := mul(x, x)
                    if iszero(eq(div(xx, x), x)) { revert(0,0) }
                    let xxRound := add(xx, half)
                    if lt(xxRound, xx) { revert(0,0) }
                    x := div(xxRound, base)
                    if mod(n,2) {
                        let zx := mul(z, x)
                        if and(iszero(iszero(x)), iszero(eq(div(zx, x), z))) { revert(0,0) }
                        let zxRound := add(zx, half)
                        if lt(zxRound, zx) { revert(0,0) }
                        z := div(zxRound, base)
                    }
                }
            }
        }
    }

    function rmul(uint x, uint y) internal pure returns (uint z) {
        z = mul(x, y) / ONE;
    }

    function add(uint x, uint y) internal pure returns (uint z) {
        require((z = x + y) >= x);
    }

    function sub(uint x, uint y) internal pure returns (uint z) {
        require((z = x - y) <= x);
    }

    function mul(uint x, uint y) internal pure returns (uint z) {
        require(y == 0 || (z = x * y) / y == x);
    }

    // // --- Administration ---
    function file(bytes32 what, uint256 data) external auth {
        require(live == 1, "Pot/not-live");
        // require(now == rho, "Pot/rho-not-updated");
        if (what == "dsr") dsr = data;
        else revert("Pot/file-unrecognized-param");
    }

    // function file(bytes32 what, address addr) external auth {
    //     if (what == "vow") vow = addr;
    //     else revert("Pot/file-unrecognized-param");
    // }

    function cage() external auth {
        live = 0;
        dsr = ONE;
    }

    // --- Savings Rate Accumulation ---
    function drip(uint256 timeout) external returns (uint tmp) {
        // require(now >= rho, "Pot/invalid-now");
        tmp = rmul(rpow(dsr, timeout, ONE), chi);
        // uint chi_ = sub(tmp, chi);
        chi = tmp;
        // rho = now;
        // vat.suck(address(vow), address(this), mul(Pie, chi_));
    }

    // --- Savings Dai Management ---
    function join(uint wad) external {
        // require(now == rho, "Pot/rho-not-updated");
        pie[msg.sender] = add(pie[msg.sender], wad);
        Pie             = add(Pie,             wad);
        // vat.move(msg.sender, address(this), mul(chi, wad));
        gains[msg.sender] -= int(mul(chi, wad));
    }

    function exit(uint wad) external {
        pie[msg.sender] = sub(pie[msg.sender], wad);
        Pie             = sub(Pie,             wad);
        // vat.move(address(this), msg.sender, mul(chi, wad));
        gains[msg.sender] += int(mul(chi, wad));
    }

    function resetgain() external {
        gains[msg.sender] = 0;
    }
}

I execute in the following order:

  1. echo $(bc -l <<< "scale=27; e( l(2 / 100 + 1)/(60 _ 60 _ 24 _ 365)) _ 10^27") ==> dsr 2%
  2. file(0x6473720000000000000000000000000000000000000000000000000000000000, 1000000000627937192491029810)
  3. join(100000000000000000000) //join 100 Dai at T1
  4. drip(31536000) // After 1 year (86400 * 365)
  5. exit(100000000000000000000) //exit 100 Dai at T2
  6. gains[msg.sender] ===> 1999999999999999997283187900000000000000000000

negligible error, gains[msg.sender] is 2, I think this is correct. But then I put in 100 Dai again:

  1. resetgain(); //reset gains[msg.sender] = 0;
  2. join(100000000000000000000) //join 100 Dai at T3
  3. drip(31536000) //After 1 year again
  4. exit(100000000000000000000) //exit 100 Dai at T4
  5. gains[msg.sender] ===> 2039999999999999997174515400000000000000000000

negligible error, gains[msg.sender] is 2.04, I don't understand why the interest has increased here, I think T1 ===> T2 is 100 Dai annual compounding, t3 ===> t4 is also 100 Dai annual compounding, Why interest is different? The later you join, the higher the interest?

Reentrancy detected

Reentrancy in Flopper.deal(uint256) (flop.sol#133-139):
External calls:
- gem.mint(bids[id].guy,bids[id].lot) (flop.sol#137)
State variables written after the call(s):
- bids (flop.sol#138)
Reentrancy in Flopper.dent(uint256,uint256,uint256) (flop.sol#117-132):
External calls:
- vat.move(msg.sender,bids[id].guy,bid) (flop.sol#127)
State variables written after the call(s):
- bids (flop.sol#129)
- bids (flop.sol#130)
- bids (flop.sol#131)
Reentrancy in Flopper.yank(uint256) (flop.sol#144-149):
External calls:
- vat.move(address(this),bids[id].guy,bids[id].bid) (flop.sol#147)
State variables written after the call(s):
- bids (flop.sol#148)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-1

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.