Skip to content

Conversation

@danielradu10
Copy link
Contributor

Reasoning behind the pull request

  • added unit tests for merging state accesses for same account

Proposed changes

Testing procedure

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

c.AddStateAccess(&data.StateAccess{
Type: data.Write,
TxHash: []byte("hash"),
MainTrieKey: []byte(strconv.Itoa(0)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Better use []byte("account1") or something like this instead of []byte(strconv.Itoa(0)). It is more readable. Change in all these tests. Use []byte(strconv.Itoa(i)) only if you add state accesses from a loop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

TxHash: []byte("hash"),
MainTrieKey: []byte(strconv.Itoa(1)),
MainTrieVal: []byte("mainTrieVal1"),
AccountChanges: &data.AccountChanges{
Copy link
Contributor

Choose a reason for hiding this comment

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

Can extract this in a variable and use that variable on L588 when you check that the same account change was returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

TxHash: []byte("txHash"),
AccountChanges: defaultAccChanges,
MainTrieVal: []byte(fmt.Sprintf("mainTrieVal%d", i)),
Operation: uint32(math.Pow(2, float64(i%6))),
Copy link
Contributor

Choose a reason for hiding this comment

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

declare a slice like

operations := []uint32{
	data.NotSet,
	data.GetCode,
	data.SaveAccount,
	data.GetAccount,
	data.WriteCode,
	data.RemoveDataTrie,
	data.GetDataTrieValue,
}

and use operations[i%len(operations)] here. It is more clear this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

replaced

@BeniaminDrasovean BeniaminDrasovean merged commit cd42a94 into merge-same-account-accessess Apr 29, 2025
2 checks passed
@BeniaminDrasovean BeniaminDrasovean deleted the unit-tests branch April 29, 2025 10:41
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.

3 participants