feat: change TablelandDeployments to use registry impl, not interface

[dtbuchholz]

Summary

Changes the TablelandDeployments contract to return the registry implementation instead of its interface, allowing for more methods to be called (e.g., safeTransferFrom). Also, a getBaseURI method was added so that a contract can fetch the Tableland Gateway URI directly from the TablelandDeployments library. Closes EVM-51.

Details

The core change uses the following imports & interface:

import {ITablelandTables} from "../interfaces/ITablelandTables.sol";
import {IERC721AUpgradeable} from "erc721a-upgradeable/contracts/IERC721AUpgradeable.sol";

interface TablelandTablesImpl is ITablelandTables, IERC721AUpgradeable {}

Then, when the get() method is called, instead of returning ITablelandTables at a deployed registry address, the returned value is TablelandTablesImpl. This allows for the methods from IERC721AUpgradeable to be called.

How it was tested

• Created a TestTablelandDeployments.sol contract, which makes calls to the TablelandDeployments.sol library and executes subsequent methods like create, mutate, setController, and safeTransferFrom.
• Tests to reflect these updates.
• Currently, there is also a method for getInterface(), which returns the original get() method's ITablelandTables interface. If you run the npm run test:gas script, you'll see the following differences:
  • create, which makes use of the TablelandTablesImpl, uses 90401 gas.
  • createWithInterface, which makes use of the ITablelandTables, uses 90423 gas. (interestingly, this uses more gas.)
  • mutate, which makes use of the TablelandTablesImpl, uses 52855 gas.
  • mutateWithInterface, which makes use of the ITablelandTables, uses 52789 gas.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[dtbuchholz]

@sanderpick wanted to put this on your radar. i have to fix the tests, but before doing so, i wanted to see if this seemed like the right approach and/or worthwhile doing.

TL;DR—the existing get() method returns the registry interface, but that means I can't access all registry methods (e.g., I can't call ERC721 methods like TablelandDeployments.get().safeTranfserFrom(...)). so, i altered get() to use interface TablelandTablesImpl is ITablelandTables, IERC721AUpgradeable {}, and i moved the existing functionality from get() to a method called getInterface()—not sure if it's worthwhile keeping or not. i also added a method for getBaseURI that returns the gateway.

[dtbuchholz]

adjusted this interface so it can be reused below—i don't think this should affect anything downstream but calling it out, in case i should create a separate interface for block polling times.

[dtbuchholz]

@sanderpick for the pollingController PR, i've added block times to networks.ts so that the SDK can use them.

i pulled these values from the go-tableland repo. e.g., if a chain has NewBlockPollFreq of 10s and MinBlockDepth of 1, the value used below would be 20_000. or for OP and Arbitrum, we have 5s polling and zero block depth, so i'm using 5_000 as the value.

but, now that i think about it...in reality, the polling frequencies from go-tableland aren't actually the block times, so i guess these values could be misleading. e.g., FIL is 30 second blocks with 5 block depth. instead, maybe the polling periods below might want to be:

b * (d + 1) + b

where b is block time and d is depth. we need d+1 to consider the block the tx is mined in, and the addition of b at the end is a margin of safety. thus:

• FIL would have a value of 210_000 (30*6+30)
• ethereum would be 40_000 (13.5*2+13.5)
• matic would be 15_000 (5*2+5)
• all other L2s would be 10_000 (5*1+5)
• keep local as 5_000

anyways, i figure the usage with the polling controller would be something like this—and i'm not sold on the name blockPollingTimes fwiw:

import { helpers } from "@tableland/sdk";
// This import would be done under the hood within the SDK so that chains have default times, but shown here an as example
import { blockPollingTimes } from "@tableland/evm"; 

// For Filecoin testnet
const controller = helpers.createPollingController(blockPollingTimes["filecoin-calibration"], 1500); // polling timeout and interval

[dtbuchholz]

note: i ended up adjusting things with the logic defined above.

[joewagner]

Making sure I understand here. These values are the maximum amount of time a client should poll the validator for a table txn receipt?
If so, what about the name validatorPollingTimeouts?

[dtbuchholz]

@joewagner correct, i like that better! updated.

[sanderpick]

in the past I've used 10m timeout for filecoin. not sure where that came from exactly, but I think it was the "finality" that we are considering in the validator. maybe i was overestimating.

anyway, this looks good to me!

[dtbuchholz]

(i'll keep an eye on if anyone mentions FIL issues in the future wrt the SDK abort signal fix...at least for filecoin calibration, it was working w/ these timeouts. can always increase as needed.)