PrisonArt / restore

Restoring human dignity one piece at a time
https://pr1s0n.art
MIT License
6 stars 1 forks source link

Review settle functionality #10

Open hollygrimm opened 2 years ago

andytudhope commented 2 years ago

I have created a new branch for multi-auctions with an updated Justice.sol + tests. The biggest changes are (i) passing in uint256 auctionId to createBid() and settledAuction() and (ii) an additional uint256 auctionId being emitted in the AuctionCreated event.

This shouldn't be too much work to integrate (though it is some, because it necessitates updating our subgraphs too).

However, there is a severe trade-off at work here: https://github.com/PrisonArt/restore/blob/multi-auctions/packages/hardhat-ts/contracts/Justice.sol#L111

We have to make createBid() more expensive in terms of gas because we're no longer able to work with the latest auction simply in memory... I will look at trying to estimate gas so that I can report more exact numbers here.

For this reason, I'm kinda against doing it, though it does provide us flexibility and gives us the chance to run many auctions all at once, without needing to settle before beginning anew. That said, it is possible to simply deploy this version of Justice.sol, point Restore.sol at it instead, and update the frontend code when/if it ever becomes necessary (though I am also aware that not doing it now and trying to remember everything and pick up the context we now have in our on RAM will be difficult if this is something we only return to in a few years

andytudhope commented 2 years ago

OK, gas reporter works well. For multi-auctions, with use of storage:

·----------------------------------------------------|----------------------------|-------------|-----------------------------·
|                Solc version: 0.8.7                 ·  Optimizer enabled: false  ·  Runs: 200  ·  Block limit: 30000000 gas  │
·····················································|····························|·············|······························
|  Methods                                                                                                                    │
····················|································|··············|·············|·············|···············|··············
|  Contract         ·  Method                        ·  Min         ·  Max        ·  Avg        ·  # calls      ·  usd (avg)  │
····················|································|··············|·············|·············|···············|··············
|  Justice          ·  createAuction                 ·      158860  ·     175960  ·     174645  ·           13  ·          -  │
····················|································|··············|·············|·············|···············|··············
|  Justice          ·  createBid                     ·       58918  ·     135589  ·      90721  ·           12  ·          -  │
····················|································|··············|·············|·············|···············|··············
|  Justice          ·  settleAuction                 ·       65837  ·     125535  ·     101656  ·            5  ·          -  │

For auctions as they currently are, only in memory:

·---------------------------------------|----------------------------|-------------|-----------------------------·
|          Solc version: 0.8.7          ·  Optimizer enabled: false  ·  Runs: 200  ·  Block limit: 30000000 gas  │
········································|····························|·············|······························
|  Methods                                                                                                       │
····················|···················|·············|··············|·············|···············|··············
|  Contract         ·  Method           ·  Min        ·  Max         ·  Avg        ·  # calls      ·  usd (avg)  │
····················|···················|·············|··············|·············|···············|··············
|  Justice          ·  createAuction    ·      80015  ·      153215  ·     147584  ·           13  ·          -  │
····················|···················|·············|··············|·············|···············|··············
|  Justice          ·  createBid        ·      65699  ·      142371  ·      96888  ·           13  ·          -  │
····················|···················|·············|··············|·············|···············|··············
|  Justice          ·  settleAuction    ·      63385  ·      105983  ·      88944  ·            5  ·          -  │