Skip to content

Conversation

@bolekk
Copy link
Contributor

@bolekk bolekk commented Aug 4, 2022

Adding two new tasks: hexencode and base64encode
Docs in: smartcontractkit/documentation#909

@bolekk bolekk requested a review from pinebit August 4, 2022 23:02
@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2022

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2022

Solana Smoke Test Results

1 tests   1 ✔️  4m 58s ⏱️
1 suites  0 💤
1 files    0

Results for commit 19d9f38.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2022

EVM Smoke Test Results

52 tests   22 ✔️  7m 44s ⏱️
  1 suites  30 💤
  1 files      0

Results for commit 19d9f38.

♻️ This comment has been updated with latest results.

return Result{Error: errors.Wrap(err, "task inputs")}, runInfo
}

if strValue, ok := inputs[0].Value.(string); ok {
Copy link
Contributor

@pinebit pinebit Aug 5, 2022

Choose a reason for hiding this comment

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

I think this approach is wrong, because it will not support expressions like $(other_task.result).
In other tasks you could find statements like this:
errors.Wrap(ResolveParam(&input, From(VarExpr(t.Input, vars), Input(inputs, 0))), "input"),
Here, VarExpr is handling the case of expressions. However, this requires to know the exact variable type.
At the same time we do not support types like "variant".
I would suggest to limit the supported types to:

  • string
  • []byte
  • uint/int of arbitrary precision

Then you can do a chain of if-else for errors.Wrap(ResolveParam(&input, From(VarExpr(t.Input, vars), Input(inputs, 0))), "input"), trying to match the parameter type, and failing in the last else.
Would this work?

Other approach is to introduce a new param type, like VariantParam and then do what ETHABIEncodeTask2 does in convertToETHABIType function.

@bolekk bolekk force-pushed the feature/48275-create-hexencode-tasks branch 2 times, most recently from 3f8652b to 4fb776d Compare August 8, 2022 20:58
@bolekk bolekk marked this pull request as ready for review August 8, 2022 21:20
@bolekk
Copy link
Contributor Author

bolekk commented Aug 8, 2022

Docs in smartcontractkit/documentation#909

Copy link
Contributor

@pinebit pinebit left a comment

Choose a reason for hiding this comment

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

LGTM, let's just iterate once on my comments. Thank you!

pinebit
pinebit previously approved these changes Aug 9, 2022
@bolekk bolekk force-pushed the feature/48275-create-hexencode-tasks branch from 4fb776d to 7cb286b Compare August 9, 2022 14:55
pinebit
pinebit previously approved these changes Aug 9, 2022
@bolekk bolekk force-pushed the feature/48275-create-hexencode-tasks branch from 7cb286b to c18a94c Compare August 9, 2022 15:26
@bolekk bolekk force-pushed the feature/48275-create-hexencode-tasks branch from c18a94c to 19d9f38 Compare August 9, 2022 15:28
@bolekk bolekk merged commit 69ebe6b into develop Aug 9, 2022
@bolekk bolekk deleted the feature/48275-create-hexencode-tasks branch August 9, 2022 15:59
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