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

After audit changes other owner #170

Closed gbalabasquer closed 5 years ago

gbalabasquer commented 6 years ago

@NiklasKunkel @r001 very simple change discussed with Bartek and Rain about adding the possibility to set another owner to the offer. This would be useful if a proxy is creating the offer for you.

r001 commented 6 years ago

It's fine with me.

BrendanChou commented 5 years ago

Allowing for the zero address to default to msg.sender will save roughly 3000 gas per call

gbalabasquer commented 5 years ago

Allowing for the zero address to default to msg.sender will save roughly 3000 gas per call

hey, I'm not following you on this, what is exactly the proposal?

BrendanChou commented 5 years ago

Because (non-zero data bytes) cost more than (zeroed data bytes) in the call data, if you require users to start specifying their own addresses in the calldata, then this will cost additional gas

68 gas per byte of nonzero-data 4 gas per byte of zero-data

It would be nice if the zero address defaulted to msg.sender

gbalabasquer commented 5 years ago

Hmmm but it has the functions without owner param as well, which makes an internal call defaulting to msg.sender. I guess that is the same effect in gas cost.

BrendanChou commented 5 years ago

Ah okay thought these modified the existing functions rather than adding new ones, carry on

gbalabasquer commented 5 years ago

@NiklasKunkel I'm merging this, if you have any suggested change, we can do it in the branch. Anyway it is still a work in progress.