Skip to content

Test different usage of the sequence and ticket numbers.#35

Merged
dzmitryhil merged 2 commits intomasterfrom
dzmitryhil/usded-seq-ticket-tx-submittion
Oct 23, 2023
Merged

Test different usage of the sequence and ticket numbers.#35
dzmitryhil merged 2 commits intomasterfrom
dzmitryhil/usded-seq-ticket-tx-submittion

Conversation

@dzmitryhil
Copy link
Contributor

@dzmitryhil dzmitryhil commented Oct 19, 2023

Description

Test different usage of the sequence and ticket numbers.
If you use the used ticket, passed sequence, or future sequence the TX won't be accepted.

Reviewers checklist:

  • Try to write more meaningful comments with clear actions to be taken.
  • Nit-picking should be unblocking. Focus on core issues.

Authors checklist

  • Provide a concise and meaningful description
  • Review the code yourself first, before making the PR.
  • Annotate your PR in places that require explanation.
  • Think and try to split the PR to smaller PR if it is big.

This change is Reviewable

@dzmitryhil dzmitryhil requested a review from a team as a code owner October 19, 2023 06:10
@dzmitryhil dzmitryhil requested review from miladz68, wojtek-coreum and ysv and removed request for a team October 19, 2023 06:10
Copy link
Contributor

@miladz68 miladz68 left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @wojtek-coreum and @ysv)

ysv
ysv previously approved these changes Oct 19, 2023
Copy link
Collaborator

@ysv ysv left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @wojtek-coreum)

Copy link
Collaborator

@wojtek-coreum wojtek-coreum left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dzmitryhil)


integration-tests/xrpl/rpc_test.go line 504 at r1 (raw file):

	signer1 := chains.XRPL.Multisign(t, &createTicketsTx, signer1Acc)

	createTicketsTx = buildCreateTicketsTxForMultiSigning(ctx, t, chains.XRPL, ticketsToCreate, 0, nil, multisigAcc)

what's the purpose of calling the same function second time with same arguments?


integration-tests/xrpl/rpc_test.go line 505 at r1 (raw file):

	createTicketsTx = buildCreateTicketsTxForMultiSigning(ctx, t, chains.XRPL, ticketsToCreate, 0, nil, multisigAcc)
	require.NoError(t, rippledata.SetSigners(&createTicketsTx, []rippledata.Signer{

Here, you create a slice just to "deslice" it immediately


integration-tests/xrpl/rpc_test.go line 521 at r1 (raw file):

	signer1 = chains.XRPL.Multisign(t, &createTicketsTx, signer1Acc)

	createTicketsTx = buildCreateTicketsTxForMultiSigning(ctx, t, chains.XRPL, ticketsToCreate, 0, usedTicket, multisigAcc)

same here


integration-tests/xrpl/rpc_test.go line 545 at r1 (raw file):

	createTicketsTx = buildCreateTicketsTxForMultiSigning(ctx, t, chains.XRPL, ticketsToCreate, *seqNo, nil, multisigAcc)
	signer1 = chains.XRPL.Multisign(t, &createTicketsTx, signer1Acc)
	createTicketsTx = buildCreateTicketsTxForMultiSigning(ctx, t, chains.XRPL, ticketsToCreate, *seqNo, nil, multisigAcc)

... and here

Copy link
Contributor Author

@dzmitryhil dzmitryhil left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @wojtek-coreum)


integration-tests/xrpl/rpc_test.go line 504 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

what's the purpose of calling the same function second time with same arguments?

The lib we use mutates the original createTicketsTx, but we need it's original state every time.


integration-tests/xrpl/rpc_test.go line 505 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

Here, you create a slice just to "deslice" it immediately

Right, updated.


integration-tests/xrpl/rpc_test.go line 521 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

same here

Updated


integration-tests/xrpl/rpc_test.go line 545 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

... and here

Updated

Copy link
Collaborator

@wojtek-coreum wojtek-coreum left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dzmitryhil)

Copy link
Contributor

@miladz68 miladz68 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dzmitryhil)

@dzmitryhil dzmitryhil merged commit 2a73936 into master Oct 23, 2023
@ysv ysv deleted the dzmitryhil/usded-seq-ticket-tx-submittion branch June 4, 2025 18:47
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.

4 participants