Remove ITransaction interface and update Transaction to follow specs #542
Conversation
| const plainObject = { | ||
| nonce: Number(transaction.nonce), | ||
| value: transaction.value.toString(), | ||
| receiver: transaction.receiver, | ||
| sender: transaction.sender, | ||
| receiver: transaction.receiver.bech32(), |
There was a problem hiding this comment.
The new method, toBech32()?
There was a problem hiding this comment.
Also below / in other places.
| gasPrice?: IGasPrice; | ||
| gasLimit?: IGasLimit; | ||
| chainID: IChainID; | ||
| nonce?: bigint; |
There was a problem hiding this comment.
Legacy functions from transferTransactionsFactory (that use the now-removed gasEstimator) should be dropped (in a next PR).
src/transactionOnNetwork.ts
Outdated
| return { | ||
| nonce: Number(transaction.nonce), | ||
| value: transaction.value.toString(), | ||
| receiver: transaction.receiver, | ||
| sender: transaction.sender, | ||
| receiver: transaction.receiver.bech32(), |
There was a problem hiding this comment.
Can use the "new" function (toBech32).
src/transactionComputer.ts
Outdated
| senderUsername: this.toBase64OrUndefined(transaction.senderUsername), | ||
| receiverUsername: this.toBase64OrUndefined(transaction.receiverUsername), | ||
| gasPrice: Number(transaction.gasPrice), | ||
| gasLimit: Number(transaction.gasLimit), | ||
| data: this.toBase64OrUndefined(transaction.data), | ||
| data: transaction.data.length > 0 ? Buffer.from(transaction.data).toString("base64") : undefined, |
There was a problem hiding this comment.
Can be changed back to use "toBase64OrUndefined"?
src/transaction.ts
Outdated
| const hasGuardianSignature = this.guardianSignature.length > 0; | ||
| return this.getOptions().isWithGuardian() && hasGuardian && hasGuardianSignature; | ||
| const isGuardedSet = (this.options & TRANSACTION_OPTIONS_TX_GUARDED) == TRANSACTION_OPTIONS_TX_GUARDED; |
There was a problem hiding this comment.
Can we somehow use the transaction computer's hasOptionsSetFor...?
src/transactionComputer.ts
Outdated
| receiver: transaction.receiver.bech32(), | ||
| sender: transaction.sender.bech32(), |
src/transactionOnNetwork.ts
Outdated
| receiver: transaction.receiver.bech32(), | ||
| sender: transaction.sender.bech32(), |
| receiver: transaction.receiver.bech32(), | ||
| sender: transaction.sender.bech32(), |
There was a problem hiding this comment.
could've used toBech32(), but it's fine since this will be deleted.
| const sender = Address.fromBech32("erd18s6a06ktr2v6fgxv4ffhauxvptssnaqlds45qgsrucemlwc8rawq553rt2"); | ||
| const delegationContract = Address.fromBech32("erd1qqqqqqqqqqqqqqqpqqqqqqqqqqqqqqqqqqqqqqqqqqqqqtllllls002zgc"); | ||
| const sender = Address.newFromBech32("erd18s6a06ktr2v6fgxv4ffhauxvptssnaqlds45qgsrucemlwc8rawq553rt2"); | ||
| const delegationContract = Address.newFromBech32( |
There was a problem hiding this comment.
I think we should use the hex representation of built-in smart contract addresses.
| @@ -334,29 +333,29 @@ export class DelegationTransactionsFactory { | |||
| }).build(); | |||
| } | |||
|
|
|||
| createTransactionForWithdrawing(_sender: IAddress, _options: resources.ManageDelegationContractInput): Transaction { | |||
| createTransactionForWithdrawing(_sender: Address, _options: resources.ManageDelegationContractInput): Transaction { | |||
There was a problem hiding this comment.
I don't remember why this method and the ones bellow are not implemented.
There was a problem hiding this comment.
they were not added when added the entrypoint and said will do a different pr for them
| nonce: 91, | ||
| value: TokenTransfer.egldFromAmount(10), | ||
| nonce: 91n, | ||
| value: TokenTransfer.egldFromAmount(10).amount, |
There was a problem hiding this comment.
we shoudn't use this method
There was a problem hiding this comment.
I'll update in a different pr if that's ok
| @@ -40,16 +39,16 @@ describe("test transaction", function () { | |||
| let transactionOne = new Transaction({ | |||
| sender: alice.address, | |||
| receiver: bob.address, | |||
| value: TokenTransfer.egldFromAmount(42), | |||
| gasLimit: network.MinGasLimit, | |||
| value: TokenTransfer.egldFromAmount(42).amount, | |||
There was a problem hiding this comment.
I'll update in a different pr if that's ok
| @@ -289,7 +289,7 @@ export class SmartContract implements ISmartContract { | |||
| * @param nonce The owner nonce used for the deployment transaction | |||
| */ | |||
| static computeAddress(owner: Address, nonce: INonce): Address { | |||
| const deployer = Address.fromBech32(owner.bech32()); | |||
| const deployer = Address.fromBech32(owner.toBech32()); | |||
There was a problem hiding this comment.
here it seems that we create an Address from an Address.
No description provided.