Add data field option to type 0 transaction creator - Closes#606 #632
Conversation
| const processInputs = (amount, address) => ({ passphrase, secondPassphrase }) => | ||
| const dataFlag = { | ||
| char: 'd', | ||
| description: `Specifies the data for the transaction type 0. Takes a string. |
There was a problem hiding this comment.
I think description can be improved.
Message of UTF-8 string to attach, which has a maximum length of 64 bytes
@mitsujutsu what do you think?
There was a problem hiding this comment.
I agree, I checked on Lisk Documentation and its the same so modified it
docs/transaction.md
Outdated
| @@ -121,6 +121,11 @@ ARGUMENTS | |||
| ADDRESS Address of the recipient. | |||
|
|
|||
| OPTIONS | |||
| -d, --data=data | |||
| Specifies the data for the transaction type 0. Takes a string. | |||
There was a problem hiding this comment.
This should be changed based on the flag description
| 'transaction:create:transfer', | ||
| defaultAmount, | ||
| defaultAddress, | ||
| '--data=XXXXXX', |
| ); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
we should add a case for invalid data too
There was a problem hiding this comment.
as we discussed, we don't need it
| const processInputs = (amount, address) => ({ passphrase, secondPassphrase }) => | ||
| const dataFlag = { | ||
| char: 'd', | ||
| description: `Message of UTF-8 string to attach, which has a maximum length of 64 bytes. |
There was a problem hiding this comment.
I feel like this sentence doesn't make much sense...
@mitsujutsu can help with the better english i think
There was a problem hiding this comment.
Oh I thought you wanted to use this sentence only 😄
There was a problem hiding this comment.
"Optional UTF8 encoded data (maximum of 64 bytes) to include in the transaction asset."
| const processInputs = (amount, address) => ({ passphrase, secondPassphrase }) => | ||
| const dataFlag = { | ||
| char: 'd', | ||
| description: `Message of UTF-8 string to attach, which has a maximum length of 64 bytes. |
There was a problem hiding this comment.
"Optional UTF8 encoded data (maximum of 64 bytes) to include in the transaction asset."
| @@ -81,7 +81,35 @@ describe('transaction:create:transfer', () => { | |||
| describe('transaction:create:transfer amount address', () => { | |||
| setupTest() | |||
| .command(['transaction:create:transfer', defaultAmount, defaultAddress]) | |||
| .it('should create an tranfer transaction', () => { | |||
| .it('should create a tranfer transaction', () => { | |||
| expect(transactionUtilStub.validateAddress).to.be.calledWithExactly( | |||
There was a problem hiding this comment.
"tranfer" --> "transfer"
There was a problem hiding this comment.
Haha, good catch, these were old test and these wrong articles caught my eye but I missed out this one 😅
| defaultAddress, | ||
| '--data=Testing lisk transaction data.', | ||
| ]) | ||
| .it('should create a tranfer transaction', () => { | ||
| expect(transactionUtilStub.validateAddress).to.be.calledWithExactly( |
| @@ -109,7 +137,7 @@ describe('transaction:create:transfer', () => { | |||
| defaultAddress, | |||
| '--no-signature', | |||
| ]) | |||
| .it('should create an tranfer transaction without signature', () => { | |||
| .it('should create a tranfer transaction without signature', () => { | |||
| expect(transactionUtilStub.validateAddress).to.be.calledWithExactly( | |||
| @@ -131,7 +159,7 @@ describe('transaction:create:transfer', () => { | |||
| defaultAddress, | |||
| '--passphrase=pass:123', | |||
| ]) | |||
| .it('should create an tranfer transaction', () => { | |||
| .it('should create a tranfer transaction', () => { | |||
| expect(transactionUtilStub.validateAddress).to.be.calledWithExactly( | |||
| @@ -160,7 +188,7 @@ describe('transaction:create:transfer', () => { | |||
| '--passphrase=pass:123', | |||
| '--second-passphrase=pass:456', | |||
| ]) | |||
| .it('should create an tranfer transaction', () => { | |||
| .it('should create a tranfer transaction', () => { | |||
| expect(transactionUtilStub.validateAddress).to.be.calledWithExactly( | |||
|
@shuse2 @mitsujutsu I've made changes corresponding to your review. Please review again. |
|
Closing since it's already merged |
shuse2
deleted the
606-add-data-field-option-to-type-0-transaction-creator
branch
…-0-transaction-creator Add data field option to type 0 transaction creator - Closes#606
Description
There was no flag to take data field for
0type transaction when creating one. Now it has been added as an option using-dflag.