Skip to content
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

Fixing the Locked issue #52

Merged
merged 11 commits into from
Apr 20, 2023
Merged

Fixing the Locked issue #52

merged 11 commits into from
Apr 20, 2023

Conversation

sullof
Copy link
Contributor

@sullof sullof commented Mar 17, 2023

In the first version of the contracts there was a typo. The events Locked and Unlocked have the parameter tokenId spelled as tokendId.
It is not possible to rename the parameter without losing the possibility of reading the old parameters which are necessary to get exact info about the date when the locked status changed.
The solution is to have three events:

Locked(uint256 tokendId)
Unlocked(uint256 tokendId)

and

Locked(uint256 tokenId, bool locked)

The latter will be used from now going on when a token is locked or unlocked.
To solve the compatibility issue, we must forcefully emits Locked events to cover all the missing old events, so a new intermediate contract, EventPatch, has been add to expose two functions that allow to do so.

No need to patch the bridged versions because they have not been deployed yet.

@sullof
Copy link
Contributor Author

sullof commented Mar 18, 2023

We may remove the old events. In the end they are necessary only if there are complains. In that case we may easily build an app that retrieves the previous events from the blockchain and shows that the data are fair.
Opinions?

Copy link
Contributor

@jerrybassat7 jerrybassat7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to Remove the Old events since they are no more used. If we need them for something we can manually add them to the abi we use for what we need?

contracts/tokens/EventPatch.sol Outdated Show resolved Hide resolved
@sullof sullof merged commit c605a99 into main Apr 20, 2023
@sullof sullof deleted the fixLocked branch April 20, 2023 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve the flatten script to save the hash and the date of any flattened contract
2 participants