Revert interaction error to warning #421

popenta · 2024-03-26T14:17:10Z

Reverted the error that was throwing when interaction was used without withSender() or with Address.Zero().

Now also the transactions factories use the ArgSerializer. TransferTransactionsFactory has changed a bit, the TransactionComputer is not needed anymore by the constructor.

The nonce parameter is now optional when instantiating a Token. The default value is 0.

…ons factories

andreibancioiu · 2024-03-26T14:27:33Z

src/transactionsFactories/transferTransactionsFactory.ts


        return new TransactionBuilder({
            config: this.config!,
            sender: options.sender,
            receiver: options.receiver,
-            dataParts: [data],
+            dataParts: [Buffer.from(data).toString()],


Here might be an issue. Since dataParts wants strings - but Buffer.from(data).toString() is not necessarily a valid / utf8 string. Can we create the transaction without TransactionBuilder? Or perhaps just set the data after creating it with the builder?

I think Buffer.toString() returns by default a utf8 string.

andreibancioiu · 2024-03-26T14:29:33Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

-        this.trueAsHex = utf8ToHex("true");
-        this.falseAsHex = utf8ToHex("false");
+        this.argSerializer = new ArgSerializer();
+        this.trueAsHex = this.argSerializer.valuesToStrings([new StringValue("true")])[0];


Oh, we don't have valueToString() here. All good.

andreibancioiu · 2024-03-26T14:30:27Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

+                new BigUIntValue(options.initialSupply),
+                new BigUIntValue(options.numDecimals),


Can be merged into the call of valuesToStrings() above.

Indeed, done!

andreibancioiu · 2024-03-26T14:39:11Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

            options.canFreeze ? this.trueAsHex : this.falseAsHex,
-            utf8ToHex("canWipe"),
+            this.argSerializer.valuesToStrings([new StringValue("canWipe")])[0],


Here, above and below, for easier reading, we can do this:

const args = [ new StringValue(options.tokenName), new StringValue(options.tokenTicker), new BigUIntValue(options.numDecimals), new StringValue("canFreeze") new BooleanValue(options.canFreeze) ... and so on ] const dataParts = ["registerMetaESDT", ...this.argSerializer.valuesToStrings(args)]

andreibancioiu · 2024-03-26T14:41:17Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

-            ...(options.addRoleNFTBurn ? [utf8ToHex("ESDTRoleNFTBurn")] : []),
-            ...(options.addRoleNFTAddQuantity ? [utf8ToHex("ESDTRoleNFTAddQuantity")] : []),
-            ...(options.addRoleESDTTransferRole ? [utf8ToHex("ESDTTransferRole")] : []),
+            ...this.argSerializer.valuesToStrings([


A bit hard to read, here / above / below. Previously, code was easier to read.

Maybe do the args trick as in the comment above?

Also:

if options.addRoleNFTCreate: args.push(new StringValue("ESDTRoleNFTCreate"))

That is, ideally, we should only call valuesToStrings() once per factory function.

andreibancioiu · 2024-03-26T14:42:37Z

src/transactionsFactories/smartContractTransactionsFactory.ts

@@ -146,7 +166,11 @@ export class SmartContractTransactionsFactory {
        const args = options.args || [];
        const metadata = new CodeMetadata(isUpgradeable, isReadable, isPayable, isPayableBySmartContract);

-        let parts = ["upgradeContract", byteArrayToHex(options.bytecode), metadata.toString()];
+        let parts = [


Maybe here, keep byteArrayToHex, as before (simpler)?

Reverted the changes.

andreibancioiu · 2024-03-26T14:43:26Z

src/transactionsFactories/smartContractTransactionsFactory.ts


    constructor(options: { config: Config; abi?: IAbi; tokenComputer: TokenComputer }) {
        this.config = options.config;
        this.abi = options.abi;
        this.tokenComputer = options.tokenComputer;
        this.dataArgsBuilder = new TokenTransfersDataBuilder();
+        this.argSerializer = new ArgSerializer();


In this file, maybe the code, as it was, was all right.

andreibancioiu · 2024-03-26T14:44:01Z

src/transactionsFactories/delegationTransactionsFactory.ts

+            dataParts.push(
+                ...[
+                    options.publicKeys[i].hex(),
+                    this.argSerializer.valuesToStrings([new BytesValue(Buffer.from(options.signedMessages[i]))])[0],


We should call valuesToStrings() only once per factory method (ideally).

Now it's only used once per method.

andreibancioiu · 2024-03-26T17:10:26Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

+            "unPause",
+            this.argSerializer.valuesToStrings([new StringValue(options.tokenIdentifier)])[0],
+        ];


Ideally, without [0]. We can reuse the same pattern as above / below, even if we only have one argument.

const dataParts = [ "unPause", ...this.argSerializer.valuesToStrings([ new StringValue(options.tokenIdentifier) ]), ];

andreibancioiu · 2024-03-26T17:15:00Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

-        ];
+        const args = [new StringValue(options.tokenIdentifier), new AddressValue(options.user)];
+
+        options.addRoleNFTCreate ? args.push(new StringValue("ESDTRoleNFTCreate")) : args.push(...[]);


args.push(...[]); can be replaced with a no-operation such as 0; or null;.

https://stackoverflow.com/questions/21634886/what-is-the-javascript-convention-for-no-operation

Or, maybe, if you still want a one-liner instead of multi-line if statement:

args.push(...(condition ? [42] : []))

Replaced args.push(...[]); with 0.

andreibancioiu · 2024-03-26T17:17:19Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

-        const dataParts = ["unsetBurnRoleGlobally", utf8ToHex(options.tokenIdentifier)];
+        const dataParts = [
+            "unsetBurnRoleGlobally",
+            this.argSerializer.valuesToStrings([new StringValue(options.tokenIdentifier)])[0],


Preferably, use the same pattern as above / below, even if we have a single argument.

andreibancioiu · 2024-03-26T17:18:38Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

+            new StringValue(options.tokenTicker),
+            new BigUIntValue(options.numDecimals),
+            new StringValue("canFreeze"),
+            new StringValue(options.canFreeze ? this.trueAsString : this.falseAsString),


A private function such as boolToString() might be useful. E.g.

new StringValue("canFreeze"), new StringValue(boolToString(options.canFreeze)), ...

indeed, added.

andreibancioiu · 2024-03-26T17:20:17Z

src/transactionsFactories/delegationTransactionsFactory.ts

-        const dataParts = ["setCheckCapOnReDelegateRewards", utf8ToHex("true")];
+        const dataParts = [
+            "setCheckCapOnReDelegateRewards",
+            this.argSerializer.valuesToStrings([new StringValue("true")])[0],


Preferably, without [0] (e.g. reuse the same pattern as when having more arguments).

andreibancioiu · 2024-03-26T17:30:00Z

src/transactionsFactories/transferTransactionsFactory.ts


        return new TransactionBuilder({
            config: this.config!,
            sender: options.sender,
            receiver: options.receiver,
-            dataParts: [data],
+            dataParts: [Buffer.from(data).toString("utf8")],


If the user intends to have the tx.data as Buffer.from([10, 10]):

> Buffer.from([10, 10]).toString("utf-8") '\n\n' >

In this case, however, the final tx.data will be fine:

> Buffer.from([10, 10]).toString("base64") 'Cgo=' > Buffer.from("\n\n").toString("base64") 'Cgo=' >

But let's take the example when the user wants to send the byte 255.

// What the user wants > Buffer.from([255]).toString("base64") '/w==' // What the user gets > Buffer.from(Buffer.from([255]).toString("utf-8")).toString("base64") '77+9' >

revert interaction error to warning & use arg serializer in transacti…

fb979bf

…ons factories

popenta self-assigned this Mar 26, 2024

popenta added 2 commits March 26, 2024 16:18

bump version

4af4ce1

mark nonce as optional when instantiating a Token

45c036b

andreibancioiu reviewed Mar 26, 2024

View reviewed changes

fixes after review

c50abf9

andreibancioiu reviewed Mar 27, 2024

View reviewed changes

fixes after re-review

7122d21

bogdan-rosianu self-requested a review March 27, 2024 12:58

bogdan-rosianu approved these changes Mar 27, 2024

View reviewed changes

andreibancioiu approved these changes Mar 27, 2024

View reviewed changes

popenta merged commit abe77cf into feat/next Mar 27, 2024
1 check passed

popenta deleted the revert-error-to-warning branch March 27, 2024 14:17

This was referenced Apr 8, 2024

v13: Merge feat/next into main #347

Merged

Enforce setting "sender" (instead of emitting a console WARN) #291

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert interaction error to warning #421

Revert interaction error to warning #421

popenta commented Mar 26, 2024 •

edited

Loading

andreibancioiu Mar 26, 2024

popenta Mar 26, 2024

andreibancioiu Mar 26, 2024

andreibancioiu Mar 26, 2024

popenta Mar 26, 2024

andreibancioiu Mar 26, 2024

popenta Mar 26, 2024

andreibancioiu Mar 26, 2024

popenta Mar 26, 2024

andreibancioiu Mar 26, 2024

popenta Mar 26, 2024

andreibancioiu Mar 26, 2024

popenta Mar 26, 2024

andreibancioiu Mar 26, 2024

popenta Mar 26, 2024

andreibancioiu Mar 26, 2024

popenta Mar 27, 2024

andreibancioiu Mar 26, 2024

popenta Mar 27, 2024

andreibancioiu Mar 26, 2024

popenta Mar 27, 2024

andreibancioiu Mar 26, 2024

popenta Mar 27, 2024

andreibancioiu Mar 26, 2024

andreibancioiu Mar 26, 2024

		new BigUIntValue(options.initialSupply),
		new BigUIntValue(options.numDecimals),

Revert interaction error to warning #421

Revert interaction error to warning #421

Conversation

popenta commented Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

popenta commented Mar 26, 2024 •

edited

Loading