-
Notifications
You must be signed in to change notification settings - Fork 124
Conversation
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 is enabled ,can add an event
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.
should not modify current event, can add another event which only update but not effect
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.
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 will change the data storage layout table and break upgradeability!!!!!
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.
Since you have contract LoopringV3 is ILoopringV3, ReentrancyGuard, changing any storage layout in IloopringV3 will break upgradabilty.
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.
loopringv3 is not deployed using proxy mode
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.
because it has no initialize function
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.
will you deploy a new BlockVerifier or upgrade an existing deployment?
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.
blockverifier is the same case
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.
But you are modifying the code, so what did you mean "is the same"?
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.
sorry for my confused statement. I mean that block verifier also cannot be deployed by using proxy mode like in loopringv3 so that only way to upgrade smart contract is to deploy a new one. so the disorder of storage of it doesn't matter.
dantaik
commented
Sep 19, 2024
@letsgoustc @kl456123 I think "delayed upgrade" is a general feature to all contracts. It's supposed to be contract-agnostic, you probably just need a wrapping "owner contract" that implements a delay internally without any change to the contracts being owned.
dantaik
commented
Sep 19, 2024
I think we should schedule a call for discussion.
kl456123
commented
Sep 19, 2024
@letsgoustc @kl456123 I think "delayed upgrade" is a general feature to all contracts. It's supposed to be contract-agnostic, you probably just need a wrapping "owner contract" that implements a delay internally without any change to the contracts being owned.
Yes I also think it is absolutely feasible and might be a better way to reach the same goal
No description provided.