-
Notifications
You must be signed in to change notification settings - Fork 299
feat(sdk-coin-stx): implement verify transaction method #5750
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.
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
Conversation
5c0bc95 to
0004402
Compare
| explain.outputs[0].amount.should.equal(testData.fungibleTokenTransferTx.functionArgs[2].value); | ||
| explain.outputs[0].address.should.equal(cvToString(testData.fungibleTokenTransferTx.functionArgs[1])); | ||
| explain.outputs[0].memo.should.equal('1'); | ||
| explain.outputs[0].tokenName.should.equal(testData.fungibleTokenTransferTx.tokenName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be equal to tstx:sip6dp, but value of tokenName in fixtures is not correct. https://github.com/BitGo/BitGoJS/pull/5750/files#diff-d8557ee9dc151a0475176bd9443fe1384d09c7088ed40f6f256dd937da69a224R44
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Umm no, it is the token name that is present in the contract which is tsip6dp-token.
The tstx:sip6dp is the bitgo name.
findTokenNameByContract - we can modify this function to give either bitgo name or the contract token name, will change the code depending on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried changing the function to return bitgo name of the token, tstx:tsip6dp - but verify transaction failed (because txHex will have the contract token name)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so ideally explainTransaction is taking the txHex and deriving the data which is having outputs array. Now this outputs array should have bitgo name of the token, so we should make all the changes required for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
ba5f746 to
aebbb60
Compare
| contractName: string; | ||
| functionName: string; | ||
| functionArgs: ClarityValue[]; | ||
| tokenName?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| * @param {String} contractName | ||
| * @returns {String|Undefined} | ||
| */ | ||
| export function getTokenNameIfAssetMatchesContract( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this method should be removed, it is not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| fee: '180', | ||
| contractAddress: 'STAG18E45W613FZ3H4ZMF6QHH426EXM5QTSAVWYH', | ||
| contractName: 'tsip6dp-token', | ||
| tokenName: 'tsip6dp-token', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tokenName: 'tstx:tsip6dp',
| address: 'SN2NN1JP9AEP5BVE19RNJ6T2MP7NDGRZYST1VDF3M', | ||
| amount: '10000', | ||
| memo: '1', | ||
| tokenName: 'tsip6dp-token', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tokenName: 'tstx:tsip6dp'
| it('should fail to verify transaction with wrong token', async function () { | ||
| const txPrebuild = newTxPrebuild(); | ||
| const txParams = newTxParams(); | ||
| txParams.recipients[0].tokenName = 'tstx:tsip6dp'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
txParams.recipients[0].tokenName = 'tstx:tsip8dp'
gianchandania
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Ticket: COIN-3290