-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: ABI coder #3402
base: ns/feat/abi-parser
Are you sure you want to change the base?
feat: ABI coder #3402
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
CodSpeed Performance ReportMerging #3402 will degrade performances by 14.08%Comparing Summary
Benchmarks breakdown
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
I tested out encoding an invalid value in a struct field and the error thrown is a one-liner without much details. Did you plan on doing anything related to pretty error message printing here or will that be in a subsequent PR?
|
||
const abiInterface = new Interface(abi); | ||
const abiInterface = AbiCoder.fromAbi(abi); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const abiInterface = AbiCoder.fromAbi(abi); | |
const abiCoder = AbiCoder.fromAbi(abi); |
And all the ones below that'll fail to compile because of this renaming.
* | ||
* @throws FuelError - when the function is not found | ||
*/ | ||
public getFunction(nameOrSignatureOrSelector: string): AbiCoderFunction { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that we even need this function anymore. We can possibly reopen #1111 and remove the function. Those who want to get a function by name then can do:
const myFn = abiCoder.functions['myfn'];
This would also remove the need for functionLookup
.
* | ||
* @throws FuelError - when the configurable is not found | ||
*/ | ||
public getConfigurable(name: string): AbiCoderConfigurable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd vote for removing this method as just doing abiCoder.configurables[name]
is enough.
* | ||
* @throws FuelError - when the log is not found | ||
*/ | ||
public getLog(logId: string): AbiCoderLog { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd vote for removing this method as just doing abiCoder.logs[logId]
is enough.
}; | ||
|
||
export const encoding: Encoding = { | ||
...currentEncoding, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this spread because adding support for a new encoding version which would then become the current one would imply a breaking change to behavior because some of the spread coders would be behaving differently. I'd rather have people access the encodings via encoding.v1.enum
and encoding.v2.enum
.
|
||
import type { SupportedCoder, SupportedCoders } from './encoding-types'; | ||
|
||
export const createCoderMatcher = (coders: SupportedCoders) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that the set of encoding versions is finite, we don't need to always recalculate this and can immediately create matchers for the supported encoding versions:
const v1Matcher = createMatcher<SupportedCoder | undefined>({u8: v1.u8, ...});
export const getCoderMatcher(version: string) {
switch(version) {
case "1":
return v1Matcher;
}
}
'1': v1, | ||
}; | ||
|
||
export const encoding: Encoding = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With regards to treeshakeability, I think it's better we don't export this encoding
object but rather export the v1
, v2
, etc. encodings separately as having it this way will bundle all encodings together but the user might be only interested in v2
.
// abi/src/coder/index.ts
export { v1 as encodingV1, v2 as encodingV2 } from './somewhere;
@@ -47,7 +47,7 @@ export class Predicate< | |||
> extends Account { | |||
bytes: Uint8Array; | |||
predicateData: TData = [] as unknown as TData; | |||
interface: Interface; | |||
interface: AbiCoder; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interface: AbiCoder; | |
abiCoder: AbiCoder; |
and all the other places where this renaming would break them.
configurableConstants?: { [name: string]: unknown } | ||
) { | ||
let predicateBytes = arrayify(bytes); | ||
const abiInterface: Interface = new Interface(jsonAbi); | ||
const abiInterface: AbiCoder = AbiCoder.fromAbi(jsonAbi); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const abiInterface: AbiCoder = AbiCoder.fromAbi(jsonAbi); | |
const abiCoder: AbiCoder = AbiCoder.fromAbi(jsonAbi); |
@@ -28,14 +28,13 @@ export function getDecodedLogs<T = unknown>( | |||
*/ | |||
return receipts.reduce((logs: T[], receipt) => { | |||
if (receipt.type === ReceiptType.LogData || receipt.type === ReceiptType.Log) { | |||
const interfaceToUse = new Interface(externalAbis[receipt.id] || mainAbi); | |||
const interfaceToUse = AbiCoder.fromAbi(externalAbis[receipt.id] || mainAbi); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const interfaceToUse = AbiCoder.fromAbi(externalAbis[receipt.id] || mainAbi); | |
const abiCoderToUse = AbiCoder.fromAbi(externalAbis[receipt.id] || mainAbi); |
@@ -18,10 +19,10 @@ export interface FunctionCall { | |||
} | |||
|
|||
export const getFunctionCall = ({ abi, receipt }: GetFunctionCallProps): FunctionCall => { | |||
const abiInterface = new Interface(abi); | |||
const abiInterface = AbiCoder.fromAbi(abi); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const abiInterface = AbiCoder.fromAbi(abi); | |
const abiCoder = AbiCoder.fromAbi(abi); |
@@ -54,7 +54,7 @@ export type DeployContractResult<TContract extends Contract = Contract> = { | |||
*/ | |||
export default class ContractFactory<TContract extends Contract = Contract> { | |||
bytecode: BytesLike; | |||
interface: Interface; | |||
interface: AbiCoder; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interface: AbiCoder; | |
abiCoder: AbiCoder; |
Summary
Breaking Changes
Note
These will need to be combined into the final PR (#3085)
AbiEncoding
.INPUT_COIN_FIXED_SIZE
Encoding
from the@fuel-ts/crypto
package toBufferEncoding
.In progress
Ref
Todo tests
Document
Refactor:
fromAbi
functionationlity away from the coders.AbiEncoding
class.Checklist