Update LIP0052 - improve code readability, improve/add functions, typos #477

mjerkov · 2023-09-04T12:40:45Z

Resolves Issue #476, Issue #478, Issue #479, and Issue #483.

sergeyshemyakov

I think you have forgot to address item 4 from the issue:

In the getChainID function, the check for the nftID length is missing, while it exists in the SDK.

proposals/lip-0052.md

sergeyshemyakov

Looks good!

proposals/lip-0052.md

…TransferCrossChainInternal

… code to reflect changes

sergeyshemyakov

Couple of suggestions to be more consistent with emitting event, otherwise looks good

proposals/lip-0052.md

sergeyshemyakov

Looks good now!

proposals/lip-0052.md

…s_typos

janhack

Editorial approval

improve code readability, functions, typos

6fa40a9

mjerkov requested a review from sergeyshemyakov September 4, 2023 12:41

mjerkov self-assigned this Sep 4, 2023

mjerkov added lip modification implementation change labels Sep 4, 2023

mjerkov linked an issue Sep 4, 2023 that may be closed by this pull request

Update LIP0052 - improve code readability, correct/improve getCollectionID, getChainID and lock functions, typos #476

Closed

mjerkov requested a review from Incede September 4, 2023 13:03

sergeyshemyakov suggested changes Sep 4, 2023

View reviewed changes

proposals/lip-0052.md Outdated Show resolved Hide resolved

proposals/lip-0052.md Outdated Show resolved Hide resolved

proposals/lip-0052.md Show resolved Hide resolved

apply Sergey's comments

eaafef8

sergeyshemyakov self-requested a review September 4, 2023 14:16

sergeyshemyakov approved these changes Sep 4, 2023

View reviewed changes

mjerkov linked an issue Sep 4, 2023 that may be closed by this pull request

Remove code redundancy in transfer methods of LIP 0052 #478

Closed

mjerkov changed the title ~~Update LIP0052 - improve code readability, correct/improve getCollectionID, getChainID and lock functions, typos~~ Update LIP0052 - improve code readability, improve/add functions, typos Sep 4, 2023

refactor transfer verification

8330bda

mjerkov requested a review from sergeyshemyakov September 4, 2023 16:08

sergeyshemyakov suggested changes Sep 5, 2023

View reviewed changes

proposals/lip-0052.md Show resolved Hide resolved

proposals/lip-0052.md Show resolved Hide resolved

proposals/lip-0052.md Outdated Show resolved Hide resolved

mjerkov added 4 commits September 5, 2023 10:54

move event definitions to verify functions

b07f30a

remove messageFeeTokenID from crossChainNFTTransferParamsSchema

d7c4ff4

remove NFT store check for nftID in verifyTransferInternal and verify…

b9394ea

…TransferCrossChainInternal

remove NFT store check for nftID in destroy and unlock

ae13091

mjerkov requested a review from sergeyshemyakov September 5, 2023 09:19

mjerkov linked an issue Sep 5, 2023 that may be closed by this pull request

Remove unnecessary checks in LIP 0052 #479

Closed

mjerkov added 2 commits September 5, 2023 15:51

introduce getNFT

b674bb6

introduce getNFT, remove redundant functions and refactor rest of the…

58e4ba6

… code to reflect changes

sergeyshemyakov suggested changes Sep 7, 2023

View reviewed changes

proposals/lip-0052.md Show resolved Hide resolved

proposals/lip-0052.md Outdated Show resolved Hide resolved

proposals/lip-0052.md Outdated Show resolved Hide resolved

proposals/lip-0052.md Outdated Show resolved Hide resolved

proposals/lip-0052.md Outdated Show resolved Hide resolved

add events if NFT does not exist

876ae2c

mjerkov requested a review from sergeyshemyakov September 7, 2023 15:15

outdent

d95f441

sergeyshemyakov approved these changes Sep 8, 2023

View reviewed changes

Incede mentioned this pull request Sep 11, 2023

Remove code redundancy in transfer methods of NFT module LiskArchive/lisk-sdk#8954

Closed

Incede approved these changes Sep 12, 2023

View reviewed changes

improve CCM execution, recover and support functions

ca1bd3e

mjerkov linked an issue Sep 12, 2023 that may be closed by this pull request

Improve LIP 0052 - CCM execution, recover, and support functions #483

Closed

mjerkov requested review from Incede and sergeyshemyakov September 12, 2023 13:04

sergeyshemyakov reviewed Sep 13, 2023

View reviewed changes

proposals/lip-0052.md Show resolved Hide resolved

add condition to cross-chain transfer CCM verification

0e9b648

Incede approved these changes Sep 13, 2023

View reviewed changes

mjerkov requested a review from janhack September 13, 2023 16:09

mjerkov and others added 4 commits September 14, 2023 11:03

remove LENGTH_TOKEN_ID

87b5555

move module is not nft check to the beginning of lock function

2caa0d4

Merge branch 'main' into update_lip_0052_improve_readability_function…

9c43524

…s_typos

Perform editorial edits

99d7ff3

janhack approved these changes Sep 15, 2023

View reviewed changes

janhack merged commit dc4804f into main Sep 15, 2023

janhack deleted the update_lip_0052_improve_readability_functions_typos branch September 15, 2023 16:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update LIP0052 - improve code readability, improve/add functions, typos #477

Update LIP0052 - improve code readability, improve/add functions, typos #477

mjerkov commented Sep 4, 2023 •

edited

Loading

sergeyshemyakov left a comment

sergeyshemyakov left a comment

sergeyshemyakov left a comment

sergeyshemyakov left a comment

janhack left a comment

Update LIP0052 - improve code readability, improve/add functions, typos #477

Update LIP0052 - improve code readability, improve/add functions, typos #477

Conversation

mjerkov commented Sep 4, 2023 • edited Loading

sergeyshemyakov left a comment

Choose a reason for hiding this comment

sergeyshemyakov left a comment

Choose a reason for hiding this comment

sergeyshemyakov left a comment

Choose a reason for hiding this comment

sergeyshemyakov left a comment

Choose a reason for hiding this comment

janhack left a comment

Choose a reason for hiding this comment

mjerkov commented Sep 4, 2023 •

edited

Loading