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.

Sign up for GitHub

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

Jump to bottom

chore: added static method to duplicate predicate #3432

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+543 −0
Open

chore: added static method to duplicate predicate #3432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions packages/account/src/predicate/predicate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { JsonAbi, InputValue } from '@fuel-ts/abi-coder';

Check failure on line 1 in packages/account/src/predicate/predicate.ts

View workflow job for this annotation

GitHub Actions / Lint 

File has too many classes (2). Maximum allowed is 1
import { Interface } from '@fuel-ts/abi-coder';
import { Address } from '@fuel-ts/address';
import { ErrorCode, FuelError } from '@fuel-ts/errors';
Expand Down Expand Up @@ -48,6 +48,7 @@
bytes: Uint8Array;
predicateData: TData = [] as unknown as TData;
interface: Interface;
initialBytecode: Uint8Array;

/**
* Creates an instance of the Predicate class.
Expand All @@ -73,6 +74,7 @@
const address = Address.fromB256(getPredicateRoot(predicateBytes));
super(address, provider);

this.initialBytecode = arrayify(bytecode);
this.bytes = predicateBytes;
this.interface = predicateInterface;
if (data !== undefined && data.length > 0) {
Expand Down Expand Up @@ -147,6 +149,46 @@
return mainFn?.encodeArguments(this.predicateData) || new Uint8Array();
}

/**
* Creates a new Predicate instance from an existing Predicate instance.
* @param instance - The existing Predicate instance.
* @returns A new Predicate instance with the same bytecode, ABI and provider but with the ability to set the data and configurable constants.
*/
static fromInstance<
TData extends InputValue[] = InputValue[],
TConfigurables extends { [name: string]: unknown } | undefined = { [name: string]: unknown },
>(instance: Predicate<TData, TConfigurables>) {
return new (class {
data: TData;
configurableConstants: TConfigurables;

constructor() {
this.data = instance.predicateData;
this.configurableConstants = {} as TConfigurables;
}

withData(data: TData): this {
this.data = data;
return this;
}

withConfigurableConstants(configurableConstants: TConfigurables): this {
this.configurableConstants = configurableConstants;
return this;
}

build(): Predicate<TData, TConfigurables> {
return new Predicate({
bytecode: instance.initialBytecode,
abi: instance.interface.jsonAbi,
provider: instance.provider,
data: this.data,
configurableConstants: this.configurableConstants,
});
}
})();
}
Comment on lines +157 to +190
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static fromInstance<
TData extends InputValue[] = InputValue[],
TConfigurables extends { [name: string]: unknown } | undefined = { [name: string]: unknown },
>(instance: Predicate<TData, TConfigurables>) {
return new (class {
data: TData;
configurableConstants: TConfigurables;
constructor() {
this.data = instance.predicateData;
this.configurableConstants = {} as TConfigurables;
}
withData(data: TData): this {
this.data = data;
return this;
}
withConfigurableConstants(configurableConstants: TConfigurables): this {
this.configurableConstants = configurableConstants;
return this;
}
build(): Predicate<TData, TConfigurables> {
return new Predicate({
bytecode: instance.initialBytecode,
abi: instance.interface.jsonAbi,
provider: instance.provider,
data: this.data,
configurableConstants: this.configurableConstants,
});
}
})();
}
static fromInstance<
TData extends InputValue[] = InputValue[],
TConfigurables extends { [name: string]: unknown } | undefined = { [name: string]: unknown },
>(instance: Predicate<TData, TConfigurables>, overrides?: PredicateParams = {}) {
return new Predicate({
bytecode: instance.initialBytecode,
abi: instance.interface.abi,
provider: instance.provider,
data: instance.data,
configurableConstants: instance.configurableConstants,
...overrides,
});
}

Would the following not be simpler? We'd need to keep a reference to the initial configurableConstants with this example, but this could be omitted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about something like this, but decide not to store constants. So you think that way is better?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be in favour of this route, but the team may have differing opinions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arboleya @Torres-ssf @danielbate will there be any other comments?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maintaining different APIs around predicate instantiation doesn't seem right to me, should be fluid from one to the next, the builder feels unnecessary IMO. I'm in favour of this change.

Additionally, does this function need to be static? Then we could avoid having to pass predicate as first param? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But in case if we have way to override any of fields via PredicateParams then for me there is no reason to have this method. Predicate can be created with constructor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason why I decided to make a builder pattern here is to encapsulate logic of overriding only constants and data while remain other fields unchangeable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what do you think? @danielbate @petertonysmith94

Copy link
Contributor

Choose a reason for hiding this comment

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

You could pick out the fields that are required to override:

overrides?: Pick<PredicateParams, 'data' | 'configurableConstants'> = {}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, that fits better. I will implement like this


/**
* Processes the predicate data and returns the altered bytecode and interface.
*
Expand Down
Loading
Loading