fix(world): dedupe functions in getWorldAbi result

[karooolis]

Dynamically fetched ABI from a deployed world returns user-created custom functions along with default world methods such as batchCall. However, it is not able to fetch system methods such as setField that should also be included in the final ABI for completion.

To include all system methods, the dynamically fetched ABI is concatenated with statically generated World's ABI, resulting in some duplicated functions.

deduplicateABI removes all the duplicate functions, and in case a duplicate is encountered, favors the ABI item that has named imports (i.e. statically generated World ABI), so that the ABI contains as much information as possible. In explorer case, for example, it's helpful to see the function parameter names when interacting with various functions.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 520e634

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[holic]

can you fill in the PR description about why this is important to do? Unclear if the side effects of duplicate entries (and seems intended given solidity supports overloads)

[karooolis]

can you fill in the PR description about why this is important to do? Unclear if the side effects of duplicate entries (and seems intended given solidity supports overloads)

Definitely, solidity overloads should be taken into account but ABI should contain no duplicates imo. Included change description.

[holic]

thinking about another approach where, instead of deduplicating an ABI as a whole, we just construct to the world a bit differently for our use case, starting with the base ABI (since it has named params) and add the functions registered on the world that we haven't seen yet (using function selectors as the "key" for uniqueness check, since its ultimately what is called)

const baseFunctionSelectors = IBaseWorldAbi.filter(isAbiFunction).map(toFunctionSelector);
const worldFunctionsAbi = worldFunctions
  .map((func) => functionSignatureToAbiItem(`function ${func.signature}`))
  .filter((abiItem) => !baseFunctionSelectors.includes(toFunctionSelector(abiItem)));

const worldAbi = [
  ...IBaseWorldAbi,
  ...worldFunctionsAbi,
];

[karooolis]

thinking about another approach where, instead of deduplicating an ABI as a whole, we just construct to the world a bit differently for our use case, starting with the base ABI (since it has named params) and add the functions registered on the world that we haven't seen yet (using function selectors as the "key" for uniqueness check, since its ultimately what is called)

const baseFunctionSelectors = IBaseWorldAbi.filter(isAbiFunction).map(toFunctionSelector);
const worldFunctionsAbi = worldFunctions
  .map((func) => functionSignatureToAbiItem(`function ${func.signature}`))
  .filter((abiItem) => !baseFunctionSelectors.includes(toFunctionSelector(abiItem)));

const worldAbi = [
  ...IBaseWorldAbi,
  ...worldFunctionsAbi,
];

I ended up liking this approach more too, somehow feels more clean. Only renamed the function to concatBaseAbi instead of deduplicate, and added a test. Note I'm purposefully not filtering functions because I'd like to return the full ABI here including errors, events, etc. It can be filtered on the consumer-side however is needed imo.

[holic]

is as Abi casting necessary?

[karooolis]

Without it, even with type predicate, typescript fails to recognize in the .map(..) that it's of type AbiFunction

[holic]

we could prob improve the return type of functionSignatureToAbiItem so we don't have to cast this

[karooolis]

Adjusted to return AbiFunction but in order for that to work, have to ensure that it indeed returns function ABI item, otherwise the function throws. Not sure how else to handle it