Conversation
wojtek-coreum
left a comment
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @miladz68 and @ysv)
ysv
left a comment
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil and @miladz68)
relayer/processes/xrpl_tx_observer.go line 269 at r1 (raw file):
// Any tem code Final unless the protocol changes to make the transaction valid. // tefPAST_SEQ Final when another transaction with the same sequence number is included in a validated ledger. // tefMAX_LEDGER Final when a validated ledger has a ledger index higher than the transaction's LastLedgerSequence field, and no validated ledger includes the transaction.
I don't fully understand this
lets probably discuss after call
relayer/processes/xrpl_tx_observer.go line 272 at r1 (raw file):
func txIsFinal(tx rippledata.TransactionWithMetaData) bool { txResult := tx.MetaData.TransactionResult if tx.MetaData.TransactionResult.Success() ||
nit:
if and second return is not needed here. You can just write
return tx.MetaData.TransactionResult.Success() || ...
dzmitryhil
left a comment
There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @miladz68 and @ysv)
relayer/processes/xrpl_tx_observer.go line 269 at r1 (raw file):
Previously, ysv (Yaroslav Savchuk) wrote…
I don't fully understand this
lets probably discuss after call
Sure
relayer/processes/xrpl_tx_observer.go line 272 at r1 (raw file):
Previously, ysv (Yaroslav Savchuk) wrote…
nit:
ifand secondreturnis not needed here. You can just writereturn tx.MetaData.TransactionResult.Success() || ...
Rright, updated.
wojtek-coreum
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @miladz68 and @ysv)
ysv
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @miladz68)
ysv
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! all files reviewed, all discussions resolved (waiting on @miladz68)
# Conflicts: # relayer/processes/xrpl_tx_observer.go # relayer/xrpl/constatns.go
wojtek-coreum
left a comment
There was a problem hiding this comment.
Reviewed 4 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dzmitryhil, @miladz68, and @ysv)
relayer/xrpl/constatns.go line 8 at r3 (raw file):
TecPathDryTxResult = "tecPATH_DRY" // TefNOTicketTxResult defines the result which indicates the usage of the passed ticket or not created ticket. // TefNOTicketTxResult defines that the usage of the passed ticket or not created ticket.
duplicated
dzmitryhil
left a comment
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @miladz68, @wojtek-coreum, and @ysv)
relayer/xrpl/constatns.go line 8 at r3 (raw file):
Previously, wojtek-coreum (Wojtek) wrote…
duplicated
Done.
miladz68
left a comment
There was a problem hiding this comment.
Reviewed 3 of 4 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @wojtek-coreum and @ysv)
ysv
left a comment
There was a problem hiding this comment.
Reviewed 3 of 4 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @wojtek-coreum)
wojtek-coreum
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @dzmitryhil)
Description
Add XRPL tx finality check.
XRPL tx finality spec: https://xrpl.org/finality-of-results.html
Reviewers checklist:
Authors checklist
This change is