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

[Keystone] Minor bugfixes #13436

Merged
merged 2 commits into from
Jun 6, 2024
Merged

[Keystone] Minor bugfixes #13436

merged 2 commits into from
Jun 6, 2024

Conversation

bolekk
Copy link
Contributor

@bolekk bolekk commented Jun 5, 2024

  1. Fix workflowName decoding in KeystoneFeedsConsumer.sol
  2. Fix potential panic in triggerSubscriber.UnregisterTrigger() (KS-225) and make (un)register idempotent
  3. Fix empty report check in write_target
  4. Some comment/log improvements

1. Fix workflowName decoding in KeystoneFeedsConsumer.sol
2. Fix potential panic in triggerSubscriber.UnregisterTrigger() (KS-225) and make (un)register idempotent
3. Fix empty report check in write_target
4. Some comment/log improvements
Copy link
Contributor

@krehermann krehermann left a comment

Choose a reason for hiding this comment

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

golang looks good, not sure about the solidity. why isn't there a test for it?

Comment on lines +91 to +92
// no shifting needed for bytes10 type
workflowName := mload(add(metadata, 64))
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah right, I fixed this in the forwarder but didn't realize there's a matching snippet in the consumer

@bolekk bolekk enabled auto-merge June 6, 2024 02:08
@bolekk bolekk added this pull request to the merge queue Jun 6, 2024
Merged via the queue into develop with commit f37afb9 Jun 6, 2024
106 checks passed
@bolekk bolekk deleted the bugfix/KS-256-fixes branch June 6, 2024 02:29
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