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 6312/meta tx standard #1885
En 6312/meta tx standard #1885
Conversation
# Conflicts: # epochStart/bootstrap/process_test.go
… of hex character - fixed genesis package
…etwork/elrond-go into EN-6312/Meta-tx-standard
…N-6312/Meta-tx-standard
# Conflicts: # cmd/node/main.go # epochStart/bootstrap/common.go # epochStart/bootstrap/process.go # epochStart/bootstrap/process_test.go # integrationTests/multiShard/endOfEpoch/startInEpoch/startInEpoch_test.go
… tx value should go back to relayer.
…o relayer - the difference should go back to sender.
…o relayer - the difference should go back to sender.
@@ -18,8 +20,11 @@ github.com/ElrondNetwork/elrond-vm-common v0.0.0-20191203115206-691b00a6e76a/go. | |||
github.com/ElrondNetwork/elrond-vm-common v0.1.9/go.mod h1:ZakxPST/Wt8umnRtA9gobcy3Dw2bywxwkC54P5VhO9g= | |||
github.com/ElrondNetwork/elrond-vm-common v0.1.19 h1:mRO768HtMyXY23pvG18DonKVEIlNvXxoyKP94S9fb2A= | |||
github.com/ElrondNetwork/elrond-vm-common v0.1.19/go.mod h1:ZakxPST/Wt8umnRtA9gobcy3Dw2bywxwkC54P5VhO9g= |
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.
go mod tidy?
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
@@ -279,7 +285,7 @@ func (sc *scProcessor) ExecuteSmartContractTransaction( | |||
acntDst, err = sc.reloadLocalAccount(acntDst) | |||
if err != nil { | |||
log.Debug("reloadLocalAccount error", "error", err.Error()) | |||
return nil | |||
return 0, err |
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.
Does this value has a constant? Maybe move to a (new) const.
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.
when critical error is returned the return code is not used, like sending nil back when critical error
@@ -137,6 +148,55 @@ func (inTx *InterceptedTransaction) CheckValidity() error { | |||
return nil | |||
} | |||
|
|||
func (inTx *InterceptedTransaction) verifyIfRelayedTx(tx *transaction.Transaction) error { |
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.
Tests missing for this 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.
agreed after first reviews will do it. may merge after disabling this txtype
@@ -393,6 +409,229 @@ func (txProc *txProcessor) moveBalances( | |||
return nil | |||
} | |||
|
|||
func (txProc *txProcessor) processRelayedTx( |
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 needs heavy testing
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.
agreed after first reviews will do it. may merge after disabling.
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 heavy unit-testing and a few integration tests.
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.
Some questions and / or small findings.
Nice and detailed document 💯
@@ -414,6 +414,9 @@ const BuiltInFunctionSaveKeyValue = "SaveKeyValue" | |||
// BuiltInFunctionESDTTransfer is the key for the elrond standard digital token transfer built-in function | |||
const BuiltInFunctionESDTTransfer = "ESDTTransfer" | |||
|
|||
// RelayedTransaction is the key for the elrond meta/gassless/relayed transaction standard | |||
const RelayedTransaction = "relayedTx" |
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 rename the document (the title) to use this term instead of "meta".
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.
process/errors.go
Outdated
var ErrRecursiveRelayedTXIsNotAllowed = errors.New("recursive relayed tx is not allowed") | ||
|
||
// ErrRelayedTxValueHigherThenUserTxValue signals that relayed tx value is higher then user tx value | ||
var ErrRelayedTxValueHigherThenUserTxValue = errors.New("relayed tx value is higher then user tx value") |
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.
Typo "than".
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
process/errors.go
Outdated
var ErrInvalidVMType = errors.New("invalid VM type") | ||
|
||
// ErrRecursiveRelayedTXIsNotAllowed signals that recursive relayed tx is not allowed | ||
var ErrRecursiveRelayedTXIsNotAllowed = errors.New("recursive relayed tx is not allowed") |
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.
Typo / case of "TX" vs. "Tx" below.
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
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.
Next PR should address the missing unit tests & integration tests
Implemented meta transaction standard.
Attached documentation: https://docs.google.com/document/d/1P-ZNs2BCvHheb9jAkVC6MYWsWfGoQzAHbvBnrvezXAQ/edit