Prototech 54: Consider checking token id sort order in the pool

[prateek105]

Description of change

High level

• See - https://github.com/Fixed-Point-Solutions/prototech-ajna-audit/issues/54
• Moved sorted token id and duplicate token id checks from ERC721PoolFactory to ERC721Pool to reduce gas cost for deploying pool.

Contract size

Pre Change

ERC721Pool - 24,386B (99.22%)

Post Change

ERC721Pool - 24,501B (99.69%)

Gas usage

Pre Change

| Deployment Cost                                      | Deployment Size |                   |        |                     |         |
| 5660038                                              | 28089           |                   |        |                     |         |
| Function Name                                        | min             | avg               | median | max                 | # calls |
| deployPool                                           | 1039            | 63385769223808816 | 305269 | 8937393460516718777 | 141     |

Post Change

| Deployment Cost                                      | Deployment Size |                   |        |                     |         |
| 5643671                                              | 28003           |                   |        |                     |         |
| Function Name                                        | min             | avg               | median | max                 | # calls |
| deployPool                                           | 1039            | 62065232364984430 | 300540 | 8937393460516718777 | 144     |

[prateek105]

I prefer the old implementation. The one-time gas spend on the double iteration is worth saving 115 bytes of space in the contract, and keeping the code in the factory where it belongs.

yes, agree. closing this one.

[brianmcmichael]

This suggestion was less about saving on deployment bytecode than on runtime expense, reducing of which would increase the number of token id's that could be practically enabled in a pool. The existing tests generally only run against very small subpools so you'd want to benchmark this against a larger subset to really see the gas savings in action. It may not be enough to warrant this change though.

Note that the Pool contracts can be deployed independently of the factory and without a sort order check but that's probably outside the context of Ajna's business purposes so long as everything is run through the factory, so it looks like the ordering is really only necessary in the factory for generating the hash used for cataloging purposes.