feat(accountAllowanceApproveTransaction): Implement AccountAllowanceApproveTransaction E2E tests: TCK
#314
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Rob Walworth <[email protected]>
Signed-off-by: Rob Walworth <[email protected]>
Signed-off-by: Rob Walworth <[email protected]>
rwalworth
requested review from
0xivanov,
RickyLB,
agadzhalov,
jsync-swirlds,
gsstoykov and
ivaylogarnev-limechain
Signed-off-by: Rob Walworth <[email protected]>
|
|
||
| // TODO: Get mirror node interface with OpenAPI | ||
| async getNftAllowances(accountId: string): Promise<any> { | ||
| const url = `${this.mirrorNodeRestJavaUrl}/api/v1/accounts/${accountId}/allowances/nfts`; |
There was a problem hiding this comment.
Could you please add the mirrorNodeRestJavaUrl value inside the .env examples.
Comment on lines
+66
to
+76
| let foundAllowance = false; | ||
| for (let i = 0; i < mirrorNodeInfo.allowances.length; i++) { | ||
| if ( | ||
| mirrorNodeInfo.allowances[i].owner === ownerAccountId && | ||
| mirrorNodeInfo.allowances[i].spender === spenderAccountId && | ||
| mirrorNodeInfo.allowances[i].amount.toString() === amount | ||
| ) { | ||
| foundAllowance = true; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggested change
| let foundAllowance = false; | |
| for (let i = 0; i < mirrorNodeInfo.allowances.length; i++) { | |
| if ( | |
| mirrorNodeInfo.allowances[i].owner === ownerAccountId && | |
| mirrorNodeInfo.allowances[i].spender === spenderAccountId && | |
| mirrorNodeInfo.allowances[i].amount.toString() === amount | |
| ) { | |
| foundAllowance = true; | |
| break; | |
| } | |
| } | |
| const foundAllowance = mirrorNodeInfo.allowances.some( | |
| allowance => | |
| allowance.owner === ownerAccountId && | |
| allowance.spender === spenderAccountId && | |
| allowance.amount.toString() === amount | |
| ); |
| await JSONRPCRequest(this, "reset"); | ||
| }); | ||
|
|
||
| async function verifyHbarAllowance( |
There was a problem hiding this comment.
Could you please put this function into the helper directory?
Comment on lines
+90
to
+101
| let foundAllowance = false; | ||
| for (let i = 0; i < mirrorNodeInfo.allowances.length; i++) { | ||
| if ( | ||
| mirrorNodeInfo.allowances[i].owner === ownerAccountId && | ||
| mirrorNodeInfo.allowances[i].spender === spenderAccountId && | ||
| mirrorNodeInfo.allowances[i].token_id === tokenId && | ||
| mirrorNodeInfo.allowances[i].amount.toString() === amount | ||
| ) { | ||
| foundAllowance = true; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggested change
| let foundAllowance = false; | |
| for (let i = 0; i < mirrorNodeInfo.allowances.length; i++) { | |
| if ( | |
| mirrorNodeInfo.allowances[i].owner === ownerAccountId && | |
| mirrorNodeInfo.allowances[i].spender === spenderAccountId && | |
| mirrorNodeInfo.allowances[i].token_id === tokenId && | |
| mirrorNodeInfo.allowances[i].amount.toString() === amount | |
| ) { | |
| foundAllowance = true; | |
| break; | |
| } | |
| } | |
| const foundAllowance = mirrorNodeInfo.allowances.some( | |
| allowance => | |
| allowance.owner === ownerAccountId && | |
| allowance.spender === spenderAccountId && | |
| allowance.tokenId === tokenId && | |
| allowance.amount.toString() === amount | |
| ); |
| expect(foundAllowance).to.be.true; | ||
| } | ||
|
|
||
| async function verifyTokenAllowance( |
There was a problem hiding this comment.
Could you please put this function into the helper directory?
| expect(foundAllowance).to.be.true; | ||
| } | ||
|
|
||
| async function verifyNftAllowance( |
There was a problem hiding this comment.
Could you please put this function into the helper directory?
| expect(foundAllowance).to.equal(allowanceExists); | ||
| } | ||
|
|
||
| async function verifyApprovedForAllAllowance( |
There was a problem hiding this comment.
- Could you please put this function into the helper directory?
- Do the same as the previous examples in the loop.
- I think those functions could be arrow.
Comment on lines
+1953
to
+2014
| await JSONRPCRequest(this, "approveAllowance", { | ||
| allowances: [ | ||
| { | ||
| ownerAccountId, | ||
| spenderAccountId, | ||
| nft: { | ||
| tokenId, | ||
| serialNumbers: ["1", "2", "3"], | ||
| }, | ||
| }, | ||
| ], | ||
| commonTransactionParams: { | ||
| signers: [ownerPrivateKey], | ||
| }, | ||
| }); | ||
|
|
||
| const serialNumbers = ["1", "2", "3"]; | ||
| await JSONRPCRequest(this, "approveAllowance", { | ||
| allowances: [ | ||
| { | ||
| ownerAccountId, | ||
| spenderAccountId: accountId, | ||
| nft: { | ||
| tokenId, | ||
| serialNumbers, | ||
| }, | ||
| }, | ||
| ], | ||
| commonTransactionParams: { | ||
| signers: [ownerPrivateKey], | ||
| }, | ||
| }); | ||
|
|
||
| await retryOnError(async () => { | ||
| await verifyNftAllowance( | ||
| true, | ||
| ownerAccountId, | ||
| accountId, | ||
| tokenId, | ||
| serialNumbers[0], | ||
| ); | ||
| }); | ||
|
|
||
| await retryOnError(async () => { | ||
| await verifyNftAllowance( | ||
| true, | ||
| ownerAccountId, | ||
| accountId, | ||
| tokenId, | ||
| serialNumbers[1], | ||
| ); | ||
| }); | ||
|
|
||
| await retryOnError(async () => { | ||
| await verifyNftAllowance( | ||
| true, | ||
| ownerAccountId, | ||
| accountId, | ||
| tokenId, | ||
| serialNumbers[2], | ||
| ); | ||
| }); |
There was a problem hiding this comment.
You can also move some of those helper functions to the helper directory and reuse them throughout the file wherever the same logic appears.
Repeat this logic for also:
(#1) Approves an NFT allowance to a spender account from an owner account
(#11) Approves an NFT allowance to a delegate spender account from a spender account with approved for all privileges from an owner account
Suggested change
| await JSONRPCRequest(this, "approveAllowance", { | |
| allowances: [ | |
| { | |
| ownerAccountId, | |
| spenderAccountId, | |
| nft: { | |
| tokenId, | |
| serialNumbers: ["1", "2", "3"], | |
| }, | |
| }, | |
| ], | |
| commonTransactionParams: { | |
| signers: [ownerPrivateKey], | |
| }, | |
| }); | |
| const serialNumbers = ["1", "2", "3"]; | |
| await JSONRPCRequest(this, "approveAllowance", { | |
| allowances: [ | |
| { | |
| ownerAccountId, | |
| spenderAccountId: accountId, | |
| nft: { | |
| tokenId, | |
| serialNumbers, | |
| }, | |
| }, | |
| ], | |
| commonTransactionParams: { | |
| signers: [ownerPrivateKey], | |
| }, | |
| }); | |
| await retryOnError(async () => { | |
| await verifyNftAllowance( | |
| true, | |
| ownerAccountId, | |
| accountId, | |
| tokenId, | |
| serialNumbers[0], | |
| ); | |
| }); | |
| await retryOnError(async () => { | |
| await verifyNftAllowance( | |
| true, | |
| ownerAccountId, | |
| accountId, | |
| tokenId, | |
| serialNumbers[1], | |
| ); | |
| }); | |
| await retryOnError(async () => { | |
| await verifyNftAllowance( | |
| true, | |
| ownerAccountId, | |
| accountId, | |
| tokenId, | |
| serialNumbers[2], | |
| ); | |
| }); | |
| const allowances = [ | |
| // Owner account allowing spender account. | |
| { | |
| ownerAccountId, | |
| spenderAccountId, | |
| nft: { tokenId, serialNumbers: ["1", "2", "3"] }, | |
| }, | |
| // Owner account allowing another spender account. | |
| { | |
| ownerAccountId, | |
| spenderAccountId: accountId, | |
| nft: { tokenId, serialNumbers: ["1", "2", "3"] }, | |
| }, | |
| ]; | |
| // Approve allowances in bulk | |
| for (const allowance of allowances) { | |
| await JSONRPCRequest(this, "approveAllowance", { | |
| allowances: [allowance], | |
| commonTransactionParams: { | |
| signers: [ownerPrivateKey], | |
| }, | |
| }); | |
| } | |
| // Consolidate verification for serial numbers | |
| const verifyAllowance = async (accountId, serialNumbers) => { | |
| for (const serialNumber of serialNumbers) { | |
| await retryOnError(async () => { | |
| await verifyNftAllowance(true, ownerAccountId, accountId, tokenId, serialNumber); | |
| }); | |
| } | |
| }; | |
| // Verify NFT allowances for the new account | |
| await verifyAllowance(accountId, ["1", "2", "3"]); |
Comment on lines
+232
to
+238
| await JSONRPCRequest(this, "deleteAccount", { | ||
| deleteAccountId: ownerAccountId, | ||
| transferAccountId: process.env.OPERATOR_ACCOUNT_ID as string, | ||
| commonTransactionParams: { | ||
| signers: [ownerPrivateKey], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Could you please add this part of logic to a helper function and re-use it, currently its repeated 5 times.
Comment on lines
+349
to
+362
| await JSONRPCRequest(this, "approveAllowance", { | ||
| allowances: [ | ||
| { | ||
| ownerAccountId, | ||
| spenderAccountId, | ||
| hbar: { | ||
| amount, | ||
| }, | ||
| }, | ||
| ], | ||
| commonTransactionParams: { | ||
| signers: [ownerPrivateKey], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Could you please add this part of logic to a helper function and re-use it, currently its repeated 4 times.
2 tasks
|
Tests are passing in JS! 🎉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This PR implements the
AccountAllowanceApproveTransactiontests documented indocs/test-specifications/crypto-service/AccountAllowanceApproveTransaction.md.Related issue(s):
#313
Checklist