-
Notifications
You must be signed in to change notification settings - Fork 711
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
Update Specs to Include Gas Price changes #546
Conversation
@@ -212,14 +212,14 @@ def max_gas(tx) -> int: | |||
if tx.type == TransactionType.Script: | |||
gas = gas + tx.gasLimit | |||
return gas | |||
|
|||
|
|||
def reserved_feeBalance(tx, assetId) -> int: |
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.
I couldn't find any function with this name in our code, and it sounds like max_fee
, so I changed it. Might be wrong.
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.
Yeah, in the code analog is max_fee
function=) Could you also update other places here(reserved_fee_balance
), please?
@@ -212,14 +212,14 @@ def max_gas(tx) -> int: | |||
if tx.type == TransactionType.Script: | |||
gas = gas + tx.gasLimit | |||
return gas | |||
|
|||
|
|||
def reserved_feeBalance(tx, assetId) -> int: |
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.
Yeah, in the code analog is max_fee
function=) Could you also update other places here(reserved_fee_balance
), please?
src/tx-format/policy.md
Outdated
|
||
Transaction is invalid if: | ||
|
||
- `max_fee` is not set |
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.
Maybe it is better to move this check to where the gas price was.
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.
I'm not sure I understand. Move this section to where the ## GasPrice
section was? Which "gas price" are you referring to?
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 also need to update unavailable_balance
function to use max_fee
policy instead of the maxFee
(reserved_fee_balance
) function
#546
This includes the changes for:
gas_price
from txgas_price
onMint
txGasPrice
policyTip
policyMaxFee
mandatorySince there is no concept of the client API in these specs, and also the implementation of tx checks, I didn't include:
gas_price
estimate or latestgas_price
API endpointsChecked<Tx>
That's fine if we don't want the scope of the VM to include those sort of things, but it's not clear to me what these specs are constrained by.