Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore!: support new ABI format #2747

Closed
wants to merge 20 commits into from
Closed

Conversation

nedsalk
Copy link
Contributor

@nedsalk nedsalk commented Jul 10, 2024

Release notes

In this release, we:

  • Added support for the new ABI specification

Summary

This PR adds support for the new ABI specification.

Breaking Changes

TBD

Checklist

  • I addedtests to prove my changes
  • I updated — all the necessary docs
  • I reviewed — the entire PR myself, using the GitHub UI
  • I described — all breaking changes and the Migration Guide

@nedsalk nedsalk added the chore Issue is a chore label Jul 10, 2024
@nedsalk nedsalk added this to the 0.x mainnet milestone Jul 10, 2024
@nedsalk nedsalk self-assigned this Jul 10, 2024
Base automatically changed from ns/chore/share-interfaces-abi to master July 11, 2024 08:36
Copy link
Contributor

Coverage Report:

Lines Branches Functions Statements
79.31%(-0.1%) 71.7%(+0.03%) 76.97%(-0.06%) 79.37%(-0.11%)
Changed Files:
Ok File (✨=New File) Lines Branches Functions Statements
🔴 ✨ packages/abi-typegen/src/transform-abi-mapper.ts 85.71%
(+85.71%)
80%
(+80%)
85.71%
(+85.71%)
85.71%
(+85.71%)
🔴 ✨ packages/abi-typegen/src/transform-abi.ts 0%
(+0%)
100%
(+100%)
0%
(+0%)
0%
(+0%)

Comment on lines +3 to +4
"abiVersion": "1",
"specVersion": "1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the new spec and incoming PR:

  • the abiVersion property was removed (unnecessary)
  • the encoding was renamed to encodingVersion (can be done in this PR as well)

We should have a basic switch approach (or similar) in place to lock the pattern for dealing with [future] different versions of specVersion, such as we have for encodingVersion.

@danielbate Thoughts?

/**
* Retrieves the appropriate encoding function for a given encoding version.
*
* @param encoding - the version to provide a strategy for.
* @throws for an unsupported encoding version.
* @returns the appropriate encoding strategy.
*/
export function getCoderForEncoding(encoding: EncodingVersion = ENCODING_V1): GetCoderFn {
switch (encoding) {
case ENCODING_V1:
return getCoderV1;
default:
throw new FuelError(
ErrorCode.UNSUPPORTED_ENCODING_VERSION,
`Encoding version ${encoding} is unsupported.`
);
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A switch/strategy SGTM. It's been sound through development.

@nedsalk
Copy link
Contributor Author

nedsalk commented Jul 23, 2024

Superseded by:

@nedsalk nedsalk closed this Jul 23, 2024
@arboleya arboleya deleted the ns/chore/support-new-abi-format branch July 23, 2024 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Issue is a chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adapt ABI format to new specification
3 participants