-
Notifications
You must be signed in to change notification settings - Fork 33
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
Non evm support and additional ccr functions integration #422
Conversation
Updated address params in payload to bytes32 in CCR functions
…03-enable-updatechannelmeta-ccr
403 address to bytes32
…ta-ccr Add CCR support for updateChannelMeta
…CR-support-for-UpdateChannelState
…ngs-ccr feat: add ccr support for createChannelSettings fn
…CR-support-for-UpdateChannelState
…nelState Add CCR for UpdateChannelState
fix test cases and structure
@@ -11,7 +11,7 @@ contract PushCoreStorageV2 { | |||
); | |||
|
|||
mapping(address => uint256) public nonces; // NOT IN USE Anymore | |||
mapping(address => uint256) public channelUpdateCounter; | |||
mapping(address => uint256) public oldChannelUpdateCounter;//NOT in use anymore |
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.
We directly reject the use of old ChannelUpdateCounter
mapping but what about the data it already stores for existing addresses ?? Has there any measures been implemented for the migration of those data?
cc: @0xNilesh
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.
two things here:
- It should first be checked if there are existing use of this
oldChannelUpdateCounter
currently? - We should include functionality for the migration of this data as well from address to bytes.
Note: Ideally, every address to bytes change should be re-checked for migration again. @0xNilesh @Zartaj0
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 might have slipped.
Yes, we planned to include its migration in the same migrate function we have for channelInfo
.
No description provided.