search

gnosis / dex-contracts

Smart contracts for the Gnosis Protocol v1
https://docs.gnosis.io/protocol/
GNU Lesser General Public License v3.0
98 stars 36 forks source link

Store auction solution hash in separate field #153

Closed bh2smith closed 5 years ago

bh2smith commented 5 years ago

Notice that the orderHash is overwritten (by the auction solution) upon successful call of applyAuction. This is needed for the purpose of snarks (and maybe for on-chain verification), but will also become problematic when implementing rollbacks. Also, I would imagine, that both the order has and solution information are necessary to be kept in storage until finalization...

https://github.com/gnosis/dex-contracts/blob/c012ee98e6034d1eaaf8445b92803429004c719c/contracts/SnappAuction.sol#L217-L218

Just wondering if we might want to take these facts into consideration.

fleupold commented 5 years ago

Good catch. I think we were lazy here and just reused the same field. We should fix this and add a separate field for the solution on the auction struct.

bh2smith commented 5 years ago

We should fix this and add a separate field for the solution on the auction struct.

Yes, I agree although this will be slightly more work to refactor (since we are currently using the PendingBatch struct from the base contract).

fleupold commented 5 years ago

Oh yeah, this is definitely a hack and needs to be changed.