Skip to content
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

imp: benchmarking v1 #44

Merged
merged 10 commits into from
Aug 17, 2024
Merged

imp: benchmarking v1 #44

merged 10 commits into from
Aug 17, 2024

Conversation

gjermundgaraba
Copy link
Contributor

No description provided.

Copy link
Member

@srdtrk srdtrk left a comment

Choose a reason for hiding this comment

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

Lgtm. I just left some comments.

Comment on lines +81 to +101
func abiEncodePacket(packet ics26router.IICS26RouterMsgsPacket) ([]byte, error) {
structType, err := abi.NewType("tuple", "", []abi.ArgumentMarshaling{
{Name: "sequence", Type: "uint32"},
{Name: "timeoutTimestamp", Type: "uint64"},
{Name: "sourcePort", Type: "string"},
{Name: "sourceChannel", Type: "string"},
{Name: "destPort", Type: "string"},
{Name: "destChannel", Type: "string"},
{Name: "version", Type: "string"},
{Name: "data", Type: "bytes"},
})
if err != nil {
return nil, err
}

args := abi.Arguments{
{Type: structType},
}

return args.Pack(packet)
}
Copy link
Member

Choose a reason for hiding this comment

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

Packet structure might change in the future, so it would be better we we could somehow use the auto-generated code here. But I will not block merging this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I wanted that as well, but I couldn't figure out how to get this from the ABI. The only alternative I found was some other weird parsing that seemed more obfuscated and error prone. This way is easy to read at least, but I do agree that it would be best if it came from the source itself. Maybe there is a way! For now, I created an issue for it: #46

test/fixtures/FixtureTest.t.sol Outdated Show resolved Hide resolved
@srdtrk srdtrk merged commit 7e6684f into main Aug 17, 2024
8 checks passed
@srdtrk srdtrk deleted the serdar/xxx-benchmarking branch August 17, 2024 05:54
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.

2 participants