-
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
Decouple calculate fee from Wallet #2354
Conversation
@@ -347,79 +346,12 @@ private Transaction MakeTransaction(DataCache snapshot, byte[] script, Signer[] | |||
tx.SystemFee = engine.GasConsumed; | |||
} | |||
|
|||
tx.NetworkFee = CalculateNetworkFee(snapshot, tx); | |||
tx.NetworkFee = CalculateNetworkFee(snapshot, tx, ProtocolSettings, (a) => GetAccount(a)?.Contract?.Script); |
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.
It seems we still need to rely on Wallet
for func CalculateNetworkFee
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.
You can use this method if you have a dictionary
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.
Looks good to me.
It opens more possibilities, in my opnion.
@shargon, my idea was that with this change we would be able to, in the future, add a new call on However, we need to discuss if this would be a good practice. |
@erikzhang Merge? |
@shargon Wasn't this fixed already? |
Close as inactive |
I want to do this, it doesn't hurt I think |
Then please update. I am not against any pr, if you update it, we can finish it. |
Moved to #3147 |
Close #2352