search

daifoundation / maker-otc

The OasisDEX protocol - Simple on-chain market for ERC20 tokens
GNU Affero General Public License v3.0
101 stars 39 forks source link

Sell all + Buy all + removing different warnings when compiling #163

Closed gbalabasquer closed 7 years ago

r001 commented 7 years ago

Guys, I have checked your wok and I have found no errors, or things I could optimize. If Nik agrees we can merge.

NiklasKunkel commented 7 years ago

So functionally everything checks, good job.

The one thing I would like to see changed is the name of the accumulator (pay_amt or buy_amt depending on the function).

We're repeating a name that already has an established behavior elsewhere. In terms of an offer, the buy_amt and pay_amt signify the amount on the make/take side remaining. If an order is partially filled buy_amt and pay_amt decrease. If an order is completely filled they implicitly go to zero.

This makes the additive mechanism of recycling these memes for use in an accumulator confusing (to me).

Does this bother anyone else or am I looking at this wrong?