Refactoring DraftTransaction to TransactionNext #376

ramonalobont · 2024-02-07T11:41:13Z

No description provided.

popenta · 2024-02-09T08:09:08Z

src/transactionsFactories/smartContractTransactionsFactory.ts

@@ -32,6 +32,7 @@ export class SmartContractTransactionsFactory {
 private readonly abiRegistry?: Abi;
 private readonly tokenComputer: TokenComputer;
 private readonly dataArgsBuilder: TokenTransfersDataBuilder;
+ //private dataParts!: string[];


remove this comment

popenta · 2024-02-09T08:15:09Z

src/transactionsFactories/smartContractTransactionsFactory.ts

- dataParts: parts,
+ return new TransactionNext({
+ sender: options.sender.bech32(),
+ receiver: Address.fromBech32(CONTRACT_DEPLOY_ADDRESS).bech32(),


can be simply CONTRACT_DEPLOY_ADDRESS. There's no need to make it an Address just to convert it back to a str.

popenta · 2024-02-09T08:17:00Z

src/transactionsFactories/smartContractTransactionsFactory.ts

@@ -152,16 +149,17 @@ export class SmartContractTransactionsFactory {

 const preparedArgs = this.argsToDataParts(args, this.abiRegistry?.constructorDefinition);
 parts = parts.concat(preparedArgs);
+ let dataParts: string[] = parts.concat(preparedArgs);


dataParts can be const

popenta · 2024-02-09T08:19:58Z

src/transactionsFactories/smartContractTransactionsFactory.ts

@@ -185,4 +183,9 @@ export class SmartContractTransactionsFactory {
 }
 return true;
 }
+
+ private buildTransactionPayload(dataParts: string[]): TransactionPayload {


I think we don't need to use TransactionPayload anymore, the method can return UInt8Array, but it's not wrong as it is now.

popenta · 2024-02-09T08:26:03Z

src/transactionsFactories/transferTransactionsFactory.ts

+ const d = options.data || "";
+ let dataParts: string[] = [d];


This can be:

const dataParts = [options.data || ""];

popenta · 2024-02-09T09:20:06Z

src/transactionsFactories/delegationTransactionsFactory.ts

+
+ return new TransactionNext({
+ sender: options.sender.bech32(),
+ receiver: Address.fromBech32(DELEGATION_MANAGER_SC_ADDRESS).bech32(),


DELEGATION_MANAGER_SC_ADDRESS is already string, does not need to be converted to Address and then back to string.

popenta · 2024-02-09T09:26:36Z

src/transactionsFactories/delegationTransactionsFactory.ts

 if (options.publicKeys.length !== options.signedMessages.length) {
 throw new Err("The number of public keys should match the number of signed messages");
 }

 const numNodes = options.publicKeys.length;

- const dataParts = ["addNodes"];
+ let dataParts: string[] = ["addNodes"];


can be const and I think the type is inferred, but maybe this is because of the linter?

popenta · 2024-02-09T09:48:45Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

@@ -603,4 +672,19 @@ IMPORTANT!
 You are about to issue (register) a new token. This will set the role "ESDTRoleBurnForAll" (globally).
 Once the token is registered, you can unset this role by calling "unsetBurnRoleGlobally" (in a separate transaction).`);
 }
+
+ private buildTransactionPayload(dataParts: string[]): TransactionPayload {


can also return UInt8Array but this is not wrong either.

popenta · 2024-02-09T09:51:08Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

- amount: this.config.issueCost,
- }).build();
+ const data = this.buildTransactionPayload(dataParts);
+ const addDataMovementGas: boolean = true;


as this is always true, it can be removed, and the computeGasLimit() method can be implemented without this parameter.

popenta · 2024-02-09T09:52:20Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

- }).build();
+ const data = this.buildTransactionPayload(dataParts);
+ const addDataMovementGas: boolean = true;
+ const providedGasLimit: BigNumber = new BigNumber(this.config.gasLimitIssue);


can be renamed to extraGasLimit

andreibancioiu

Now I see, we should have not removed the transaction builder. Sorry for incorrectly describing the steps. They should have been:

adjust (add chain ID) and rename the transaction builder with respect to: https://github.com/multiversx/mx-sdk-py-core/blob/main/multiversx_sdk_core/transaction_factories/transaction_builder.py
use TransactionNext instead DraftTransaction
remove DraftTransaction
rename remaining variables (e.g. to not contain "draft" anymore).

That is, no other code movement should happen - the gas computation logic and so on should stay as they were.

For reference: https://github.com/multiversx/mx-sdk-py-core/tree/main/multiversx_sdk_core/transaction_factories

popenta · 2024-02-12T13:53:37Z

src/transactionsFactories/transactionNextBuilder.ts

 })
 }
-}
+}


add empty line at end of file

popenta · 2024-02-12T14:01:54Z

src/transactionsFactories/transferTransactionsFactory.ts

@@ -69,13 +71,11 @@ export class NextTransferTransactionsFactory {
 const transferArgs = this.dataArgsBuilder.buildArgsForMultiESDTNFTTransfer(
 options.receiver,
 options.tokenTransfers
- );
+ )


add ; after calling the method.

popenta · 2024-02-12T14:04:16Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

@@ -603,4 +623,4 @@ IMPORTANT!
 You are about to issue (register) a new token. This will set the role "ESDTRoleBurnForAll" (globally).
 Once the token is registered, you can unset this role by calling "unsetBurnRoleGlobally" (in a separate transaction).`);
 }
-}
+}


add empty line at end of file

popenta · 2024-02-12T14:06:19Z

src/transactionsFactories/tokenManagementTransactionsFactory.ts

@@ -120,7 +122,8 @@ export class TokenManagementTransactionsFactory {
 options.canAddSpecialRoles ? this.trueAsHex : this.falseAsHex,
 ];

- return new DraftTransactionBuilder({
+


Remove one empty line unless the formatter added it, then you can ignore the comments. Applies to all methods.

popenta · 2024-02-12T14:15:00Z

src/transactionsFactories/delegationTransactionsFactory.spec.ts

 const sender = Address.fromBech32("erd18s6a06ktr2v6fgxv4ffhauxvptssnaqlds45qgsrucemlwc8rawq553rt2");
 const delagationCap = "5000000000000000000000";
 const serviceFee = 10;
 const value = new BigNumber("1250000000000000000000");

- const draft = delegationFactory.createTransactionForNewDelegationContract({
+ const next = delegationFactory.createTransactionForNewDelegationContract({


Could be renamed to transaction. (optional) Same for the other tests.

popenta · 2024-02-12T14:20:15Z

src/transactionsFactories/delegationTransactionsFactory.ts

@@ -19,33 +18,27 @@ interface Config {
 additionalGasLimitPerValidatorNode: BigNumber.Value;
 additionalGasLimitForDelegationOperations: BigNumber.Value;
 }
-


unless the formatter removed the empty lines they can be added back.

popenta · 2024-02-12T14:23:43Z

src/transactionsFactories/delegationTransactionsFactory.ts

 private computeExecutionGasLimitForNodesManagement(numNodes: number): BigNumber.Value {
 const additionalGasForAllNodes = new BigNumber(this.config.additionalGasLimitPerValidatorNode).multipliedBy(numNodes);
 return new BigNumber(this.config.gasLimitDelegationOperations).plus(additionalGasForAllNodes);
 }
-}
+}


add empty line

popenta · 2024-02-12T14:35:34Z

src/transactionsFactories/smartContractTransactionsFactory.ts

@@ -185,4 +183,4 @@ export class SmartContractTransactionsFactory {
 }
 return true;
 }
-}
+}


add empty new line

ramonalobont added 3 commits February 7, 2024 13:39

refactoring draftTransaction to TransactionNext

eb8e840

refactoring draftTransaction to TransactionNext

732ddde

refactoring all

15d216e

ramonalobont requested review from popenta and andreibancioiu February 7, 2024 13:08

ramonalobont added 3 commits February 7, 2024 16:37

Removed transactionNextBuilder

0e8a0ee

refactoring + removed draft transactions + updated tests

19f3ae5

refactoring after review

39cbfa1

andreibancioiu assigned ramonalobont Feb 9, 2024

popenta reviewed Feb 9, 2024

View reviewed changes

Fixes after review.

7beb879

ramonalobont requested a review from popenta February 9, 2024 13:02

andreibancioiu reviewed Feb 12, 2024

View reviewed changes

refactoring all + transactionNextBuilder

30195c2

ramonalobont requested a review from andreibancioiu February 12, 2024 11:59

popenta reviewed Feb 12, 2024

View reviewed changes

fixes after review

599fd49

ramonalobont requested a review from popenta February 12, 2024 15:03

+ prettier

041a2f2

andreibancioiu approved these changes Feb 13, 2024

View reviewed changes

popenta approved these changes Feb 13, 2024

View reviewed changes

popenta merged commit 38d12f1 into feat/next Feb 13, 2024
1 check passed

popenta deleted the refactoring-DraftTransaction branch February 13, 2024 08:33

popenta mentioned this pull request Feb 13, 2024

v13: Merge feat/next into main #347

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring DraftTransaction to TransactionNext #376

Refactoring DraftTransaction to TransactionNext #376

ramonalobont commented Feb 7, 2024

popenta Feb 9, 2024

popenta Feb 9, 2024

popenta Feb 9, 2024

popenta Feb 9, 2024

popenta Feb 9, 2024

popenta Feb 9, 2024

popenta Feb 9, 2024

popenta Feb 9, 2024

popenta Feb 9, 2024

popenta Feb 9, 2024

andreibancioiu left a comment •

edited

Loading

popenta Feb 12, 2024

popenta Feb 12, 2024

popenta Feb 12, 2024

popenta Feb 12, 2024

popenta Feb 12, 2024

popenta Feb 12, 2024

popenta Feb 12, 2024

popenta Feb 12, 2024

Refactoring DraftTransaction to TransactionNext #376

Refactoring DraftTransaction to TransactionNext #376

Conversation

ramonalobont commented Feb 7, 2024

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

andreibancioiu left a comment • 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

andreibancioiu left a comment •

edited

Loading