Skip to content
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

En 7630/esdt sccall and invalid #2340

Merged
merged 8 commits into from
Oct 6, 2020

Conversation

sasurobert
Copy link
Contributor

@sasurobert sasurobert commented Oct 5, 2020

Implemented calling a smart contract function after ESDT.
Implemented sending back the ESDT tokens if the transaction failed cross shard.

@iulianpascalau iulianpascalau self-requested a review October 6, 2020 05:40
@ccorcoveanu ccorcoveanu self-requested a review October 6, 2020 07:26
go.mod Outdated
@@ -46,3 +46,5 @@ require (
)

replace github.com/gogo/protobuf => github.com/ElrondNetwork/protobuf v1.3.2

replace github.com/ElrondNetwork/arwen-wasm-vm => ../arwen-wasm-vm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not forget to reference a commit here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do once arwen is in master

tx data.TransactionHandler,
acntSnd, acntDst state.UserAccountHandler,
) (vmcommon.ReturnCode, error) {
builtInFuncCall bool,
) (vmcommon.ReturnCode, *vmcommon.ContractCallInput, []byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 4 output parameters. I think it would have been more extensible if an intermediate struct that aggregates all of them would have been used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should be no more changes here.

}

var vmExec vmcommon.VMExecutionHandler
vmExec, err = findVMByTransaction(sc.vmContainer, tx)
userErrorVmOutput := &vmcommon.VMOutput{ReturnCode: vmcommon.UserError}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multiline?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

results, err = sc.processVMOutput(vmOutput, txHash, tx, acntSnd, vmInput.CallType, vmInput.GasProvided)
if err != nil {
log.Trace("process vm output returned with problem ", "err", err.Error())
return vmOutput.ReturnCode, sc.ProcessIfError(acntSnd, txHash, tx, err.Error(), []byte(vmOutput.ReturnMessage), snapshot)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think so. Will run importDB to fix whatever broken

return false, vmOutput, nil
}

userErrorVmOutput := &vmcommon.VMOutput{ReturnCode: vmcommon.UserError}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multiline?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

process/smartContract/process.go Show resolved Hide resolved
go.mod Outdated
@@ -46,3 +46,5 @@ require (
)

replace github.com/gogo/protobuf => github.com/ElrondNetwork/protobuf v1.3.2

replace github.com/ElrondNetwork/arwen-wasm-vm => ../arwen-wasm-vm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove local version

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will remove once arwen is in master

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if isAsynchronousCallBack(tx) {
if isBuiltInFunction {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I see we can get away with if isBuiltInFunction {... outside of this if and remove it from line 100. There is no case where isBuiltInFunction == true and we return something else.
At line 96 we can just have if !isDestInSelfShard

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Base automatically changed from EN-7630/ESDT-new-builtInFuncs to feat/ESDT-new-arwen October 6, 2020 09:22
Copy link
Contributor

@iulianpascalau iulianpascalau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a description for this PR

@sasurobert sasurobert merged commit 8ae5913 into feat/ESDT-new-arwen Oct 6, 2020
@sasurobert sasurobert deleted the EN-7630/ESDT-sccall-and-invalid branch October 6, 2020 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants