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

feat(casting): makeHttpClient for explicit net access with cosmjs #7935

Merged
merged 3 commits into from
Jul 11, 2023

Conversation

dckc
Copy link
Member

@dckc dckc commented Jun 15, 2023

refs: #7905

stacked on #7950

Description

makeHttpClient is an alternative to the cosmjs HttpClient that uses explicit rather than ambient access to the network.

Security Considerations

facilitates use within a Compartment, for example.

In due course, I'd like other relevant APIs in the casting package to allow passing in network access likewise.

Scaling / Documentation Considerations

n/a

Testing Considerations

Also includes captureIO and replayIO test utilities to wrap fetch, record the traffic as fixtures, and then replay it from those fixtures.

@dckc dckc force-pushed the dc-cosmjs-interpose-net branch 2 times, most recently from cb7154d to c2716e2 Compare June 16, 2023 01:52
@dckc dckc mentioned this pull request Jun 16, 2023
@dckc dckc force-pushed the dc-cosmjs-interpose-net branch 2 times, most recently from 7b15a20 to 3a10d1b Compare June 20, 2023 13:38
@dckc dckc changed the title test: use explicit net access with cosmjs (SPIKE) feat(casting): makeHttpClient for explicit net access with cosmjs Jun 20, 2023
@dckc dckc changed the base branch from master to dc-vstorage-js-proto June 20, 2023 13:44
@dckc dckc requested review from michaelfig and turadg June 20, 2023 13:48
@dckc dckc force-pushed the dc-cosmjs-interpose-net branch 3 times, most recently from 21b7030 to 3ebb206 Compare June 20, 2023 13:54
@dckc dckc marked this pull request as ready for review June 20, 2023 13:55
@dckc dckc force-pushed the dc-cosmjs-interpose-net branch from 3ebb206 to d7ffad5 Compare June 20, 2023 13:57
Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

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

I love it! The response id handling could be made spec-conformant, but this is worth landing even if you don't do that.

code: -32601,
message: 'Method not found',
},
id: 1,
Copy link
Member

Choose a reason for hiding this comment

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

This id should match the one on line 18 it is replying to, in order to conform to the JSON-RPC response object spec.

},
]),
{
id: 2,
Copy link
Member

Choose a reason for hiding this comment

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

This, too, should match the body.id above.

},
]),
{
id: 753972443441,
Copy link
Member

Choose a reason for hiding this comment

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

Same.

@turadg
Copy link
Member

turadg commented Jun 20, 2023

I haven't looked closely yet but regarding the recording have you considered something off the shelf? Eg https://github.com/nock/nock#recording

@dckc
Copy link
Member Author

dckc commented Jun 20, 2023

I haven't looked closely yet but regarding the recording have you considered something off the shelf? Eg https://github.com/nock/nock#recording

I didn't; I don't expect to find anything comparable. That is: ~100 lines and no use of ambient authority. nock is clearly >>100 lines. I don't see any support for explicit rather than ambient authority.

Base automatically changed from dc-vstorage-js-proto to master June 20, 2023 16:48
@dckc dckc enabled auto-merge June 20, 2023 23:45
@turadg turadg removed their request for review June 23, 2023 23:38
@dckc dckc force-pushed the dc-cosmjs-interpose-net branch from 7ae4130 to dd56704 Compare July 11, 2023 17:18
@dckc dckc added this pull request to the merge queue Jul 11, 2023
Merged via the queue into master with commit dfc712f Jul 11, 2023
@dckc dckc deleted the dc-cosmjs-interpose-net branch July 11, 2023 18:06
mhofman pushed a commit that referenced this pull request Aug 7, 2023
feat(casting): makeHttpClient for explicit net access with cosmjs
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