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

PP-885/explore-solution-b2 #140

Merged
merged 14 commits into from
Feb 28, 2024
Merged

PP-885/explore-solution-b2 #140

merged 14 commits into from
Feb 28, 2024

Conversation

franciscotobar
Copy link
Collaborator

@franciscotobar franciscotobar commented Jan 28, 2024

What

  • Implements solution B2

Why

  • Necessary to execute a contract while deploying a smart wallet

Refs

@franciscotobar franciscotobar changed the title refactor: first attempt PP-885/explore-solution-b2 Jan 29, 2024
@franciscotobar franciscotobar force-pushed the PP-885/explore-solution-b2 branch from 52f1fca to 6be340e Compare January 29, 2024 03:29
Copy link
Collaborator

@antomor antomor left a comment

Choose a reason for hiding this comment

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

@franciscotobar Thank you for the changes. LGTM

package.json Outdated Show resolved Hide resolved
src/relayServerUtils.ts Outdated Show resolved Hide resolved
src/relayServerUtils.ts Outdated Show resolved Hide resolved
src/relayServerUtils.ts Show resolved Hide resolved
src/relayServerUtils.ts Show resolved Hide resolved
test/unit/RelayServer.test.ts Show resolved Hide resolved
test/unit/relayServerUtils.test.ts Outdated Show resolved Hide resolved
test/unit/relayServerUtils.test.ts Outdated Show resolved Hide resolved
@franciscotobar franciscotobar force-pushed the PP-885/explore-solution-b2 branch 5 times, most recently from bb4bd1e to 483389a Compare February 2, 2024 03:37
@franciscotobar franciscotobar force-pushed the PP-885/explore-solution-b2 branch 3 times, most recently from 317e6c1 to e6760c1 Compare February 13, 2024 03:06
Copy link

@raullaprida raullaprida left a comment

Choose a reason for hiding this comment

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

I have a question regarding the allowed destinations. This being always on might not have the desired outcome in regular relay servers, as they would have to whitelist every single possible destination. Assuming this is being pushed on the main branch, shouln't we add a configuration option to enable this whitelist and having it disabled by default?

src/RelayServer.ts Outdated Show resolved Hide resolved
src/relayServerUtils.ts Fixed Show fixed Hide fixed
@franciscotobar franciscotobar force-pushed the PP-885/explore-solution-b2 branch 2 times, most recently from ff5430a to a1ae292 Compare February 14, 2024 15:47
src/relayServerUtils.ts Outdated Show resolved Hide resolved
return await handler.getAcceptedContracts();
} catch (error) {
log.warn(
`Couldn't get accepted contracts from verifier ${verifier}`,

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
.

return await handler.getAcceptedTokens();
} catch (error) {
log.warn(`Couldn't get accepted tokens from verifier ${verifier}`, error);

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
.
@antomor antomor force-pushed the PP-885/explore-solution-b2 branch from aaca9a2 to cb4f8be Compare February 15, 2024 11:10
* ci: add boltz configuration

* ci: update deployment script to be used

* ci: update server version

* docs: fix swagger api
@franciscotobar franciscotobar marked this pull request as ready for review February 16, 2024 14:53
Copy link

@raullaprida raullaprida left a comment

Choose a reason for hiding this comment

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

havent checked the tests, the rest looks ok

@antomor antomor merged commit 2963dc2 into main Feb 28, 2024
3 checks passed
@antomor antomor deleted the PP-885/explore-solution-b2 branch February 28, 2024 17:18
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