-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix fee of CreateMultisigAccount #2712
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.
Anyone tested it already wrt mainnet compatibility?
@@ -41,13 +41,13 @@ partial class ApplicationEngine | |||
/// The <see cref="InteropDescriptor"/> of System.Contract.CreateStandardAccount. | |||
/// Calculates corresponding account scripthash for the given public key. | |||
/// </summary> | |||
public static readonly InteropDescriptor System_Contract_CreateStandardAccount = Register("System.Contract.CreateStandardAccount", nameof(CreateStandardAccount), 1 << 8, CallFlags.None); | |||
public static readonly InteropDescriptor System_Contract_CreateStandardAccount = Register("System.Contract.CreateStandardAccount", nameof(CreateStandardAccount), CheckSigPrice, CallFlags.None); |
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.
My gut feeling is that computationally it's more like CheckSigPrice/2
, but that needs to be measured of course.
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.
Need to be checked if it was used in mainnet. But code looks good
I agree with @shargon, changes like this have to be cautious cause it might cause new node fail to synchronize. |
This comment was marked as resolved.
This comment was marked as resolved.
We should name every hard fork, to make it more clear which fork we are working on. Not much change to make based on this pr, just add an enum with a list of hardfork names (including names of future harforks) then match them with the hardfork versions would work. |
Good idea. |
There is also a use case for private networks with "whatever is the latest" hard fork enabled, it should be easy to configure that too. |
Done.
Agree. Now you can just delete |
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.
Code looks good to me, good change!
@shargon Can you revert the commit "T4 testnet compatible with 3.2.2"? We can discuss it in a new issue or pr. |
This reverts commit 15bea61.
Fix #2710