-
Notifications
You must be signed in to change notification settings - Fork 198
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
Add async gas locking to built-in functions #2233
Conversation
} | ||
|
||
sc.penalizeUserIfNeeded(tx, txHash, vmInput.CallType, vmInput.GasProvided, vmOutput) | ||
|
||
// Restore gas set aside for the async callback, if any; this amount will | ||
// still be available to the async callback, even after being penalized for providing too much gas | ||
vmOutput.GasRemaining += lockedGas |
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.
Penalization is done in penalizeUserIfNeeded method only if: callType != vmcommon.AsynchronousCall
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.
Updated the comment.
@@ -1004,6 +1008,12 @@ func (tpn *TestProcessorNode) initInnerProcessors() { | |||
} | |||
builtInFuncs, _ := builtInFunctions.CreateBuiltInFunctionContainer(argsBuiltIn) | |||
|
|||
if len(TestBuiltinFunctions) > 0 { |
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 "if" is not really necessary
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.
Removed.
@@ -250,7 +250,7 @@ func (sc *scProcessor) doExecuteSmartContractTransaction( | |||
snapshot := sc.accounts.JournalLen() | |||
|
|||
var vmInput *vmcommon.ContractCallInput | |||
vmInput, err = sc.createVMCallInput(tx) | |||
vmInput, err = sc.createVMCallInput(tx, txHash) |
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 not the original txhash which should be needed in case of async callback.use setOriginalTxHash 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.
Correct. Replicated the behavior of setOriginalTxHash()
in sc.createVMCallInput()
.
@@ -71,6 +71,7 @@ func (sc *scProcessor) createVMCallInput(tx data.TransactionHandler) (*vmcommon. | |||
vmCallInput.CallType = callType | |||
vmCallInput.RecipientAddr = tx.GetRcvAddr() | |||
vmCallInput.Function = function | |||
vmCallInput.OriginalTxHash = txHash | |||
|
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.
Please set originaltxhash and currentxhash as well.
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.
Done.
…etwork/elrond-go into fix-callback-gas-builtin
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.
System tests passed.
Lock a specified amount of gas before executing a built-in function, in case the call is asynchronous. The locked gas will be restored after execution, or in case of error. The intention is to keep some gas aside, in order to have a minimum amount left for the async callback.
Extra additions to
integrationTests/vm/arwen/utils.go
, to allow easy extraction of info directly from SmartContractResults (TODO: add the same functionality toTestProcessorNode
).