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

bump libocr; add context #52

Merged
merged 1 commit into from
Oct 9, 2024
Merged

bump libocr; add context #52

merged 1 commit into from
Oct 9, 2024

Conversation

jmank88
Copy link
Collaborator

@jmank88 jmank88 commented Apr 25, 2024

@jmank88 jmank88 force-pushed the BCF-2887-context-propagation branch 2 times, most recently from 5fd7326 to 25fc4cb Compare August 1, 2024 16:32
@jmank88 jmank88 force-pushed the BCF-2887-context-propagation branch 3 times, most recently from 50cfc58 to 4fc7a43 Compare August 5, 2024 23:12
@jmank88 jmank88 force-pushed the BCF-2887-context-propagation branch 5 times, most recently from 7e2d94e to f0508c0 Compare August 20, 2024 15:26
@jmank88 jmank88 force-pushed the BCF-2887-context-propagation branch 4 times, most recently from 83921af to 79f6c4b Compare September 3, 2024 11:41
@jmank88 jmank88 force-pushed the BCF-2887-context-propagation branch 2 times, most recently from 7f4ec06 to 65c2f9d Compare September 9, 2024 21:25
@jmank88 jmank88 force-pushed the BCF-2887-context-propagation branch 2 times, most recently from 8f880cc to 911783c Compare September 17, 2024 21:44
@jmank88 jmank88 force-pushed the BCF-2887-context-propagation branch 3 times, most recently from ad4e313 to 5ec01ac Compare September 30, 2024 23:33
@jmank88 jmank88 force-pushed the BCF-2887-context-propagation branch from 5ec01ac to 3c01988 Compare October 7, 2024 19:16
@jmank88 jmank88 requested a review from samsondav October 8, 2024 11:58
samsondav
samsondav previously approved these changes Oct 8, 2024
Copy link
Contributor

@samsondav samsondav left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -43,7 +44,7 @@ func UnmarshalJSONStreamValue(enc *JSONStreamValue) (StreamValue, error) {

type JSONReportCodec struct{}

func (cdc JSONReportCodec) Encode(r Report, _ llotypes.ChannelDefinition) ([]byte, error) {
func (cdc JSONReportCodec) Encode(ctx context.Context, r Report, _ llotypes.ChannelDefinition) ([]byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this ctx?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same for this one:

This call goes over GRPC because the codec is implemented in the chain relayer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or maybe in this case, just because some of the interface implementations are in the chain relayers, even if this one is not. (I don't fully grok what this impl is used for)

@@ -26,7 +27,7 @@ var _ mercury.OnchainConfigCodec = StandardOnchainConfigCodec{}
// returned by EncodeValueInt192.
type StandardOnchainConfigCodec struct{}

func (StandardOnchainConfigCodec) Decode(b []byte) (mercury.OnchainConfig, error) {
func (StandardOnchainConfigCodec) Decode(ctx context.Context, b []byte) (mercury.OnchainConfig, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Again not sure why the codecs need a ctx

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This call goes over GRPC because the codec is implemented in the chain relayer.

@jmank88 jmank88 force-pushed the BCF-2887-context-propagation branch from fdff613 to f5f6ef5 Compare October 9, 2024 12:52
@jmank88 jmank88 force-pushed the BCF-2887-context-propagation branch from f5f6ef5 to 18174ed Compare October 9, 2024 12:53
@jmank88 jmank88 marked this pull request as ready for review October 9, 2024 12:54
@jmank88 jmank88 requested a review from samsondav October 9, 2024 12:59
@jmank88
Copy link
Collaborator Author

jmank88 commented Oct 9, 2024

@samsondav Updated with real chainlink-common release

@jmank88 jmank88 merged commit 8ddab7e into master Oct 9, 2024
7 checks passed
@jmank88 jmank88 deleted the BCF-2887-context-propagation branch October 9, 2024 14:14
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