-
Notifications
You must be signed in to change notification settings - Fork 2
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
Handle intra-shard relayed transactions with signal error #81
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #81 +/- ##
==========================================
+ Coverage 71.06% 71.60% +0.54%
==========================================
Files 32 33 +1
Lines 1683 1747 +64
==========================================
+ Hits 1196 1251 +55
- Misses 407 411 +4
- Partials 80 85 +5
☔ View full report in Codecov by Sentry. |
server/services/common.go
Outdated
@@ -50,6 +50,14 @@ func isZeroAmount(amount string) bool { | |||
return false | |||
} | |||
|
|||
func isZeroBigInt(value *big.Int) bool { |
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.
can you rename?
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.
Renamed to isZeroBigIntOrNil
.
return nil, err | ||
} | ||
|
||
if isZeroBigInt(&innerTx.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.
for token transfers (the other feature branch) should we care also about non native token transfers?
if yes, maybe add a TODO?
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.
Good question. Ideally, the existing logic (in feat/esdt
) that handles logs & events for ESDT transfers should be sufficient. Added an issue here, to check & confirm: #82.
} | ||
|
||
for _, event := range tx.Logs.Events { | ||
isSignalError := event.Identifier == transactionEventSignalError |
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 const from core core.SignalErrorOperation
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.
Thanks, fixed.
f524b19
While the balance changing operations are perfectly recoverable from most kinds of relayed transactions (given the smart contract results generated by the protocol), there is a case where this is only possible by also unwrapping and inspecting the inner transaction: when the relayed transaction is completely intra-shard (both the wrapping transaction and the inner transaction are intra-shard, in the same shard), and the processing of the transaction results into a contract signal error.
The PR handles this exotic case and emits an extra balance-changing operation.