-
Notifications
You must be signed in to change notification settings - Fork 24
Contributor docs #562
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
Open
theref
wants to merge
9
commits into
nucypher:main
Choose a base branch
from
theref:contribute
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Contributor docs #562
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
14eb7d9
docs: Add simple structure to `CONTRIBUTING.md`
theref 930f0b4
docs: Fix broken links and update docs command
theref 3b31f0c
docs: Add authentication section
theref b3122bc
docs: Add test code and background info for authentication providers
theref 810c6e8
docs: Add section for context parameters
theref ca8cdb8
docs: Fix broken link and some wording suggestions
theref 05a9933
docs: Improve Publishing section
theref b6379ff
docs: Add note about convential commits
theref d2d5b86
Update CONTRIBUTING.md
theref File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,26 @@ | ||
# Contribution Guide | ||
|
||
Download, install, build, and test with: | ||
- [Quick Start](#quick-start) | ||
- [Setup](#setup) | ||
- [Basic Development Commands](#basic-development-commands) | ||
- [Documentation](#documentation) | ||
- [Publishing](#publishing) | ||
- [External Api](#external-api) | ||
- [Design and Architecture](#design-and-architecture) | ||
- [Context Parameters](#context-parameters) | ||
- [Authentication Providers](#authentication-providers) | ||
# Quick Start | ||
|
||
### Setup | ||
Download and install with: | ||
|
||
```bash | ||
git clone https://github.com/nucypher/taco-web | ||
cd taco-web | ||
pnpm install | ||
``` | ||
|
||
## Development | ||
### Basic Development Commands | ||
|
||
Execute common tasks with: | ||
|
||
|
@@ -19,15 +31,153 @@ pnpm lint | |
pnpm fix | ||
``` | ||
|
||
## Documentation | ||
### Documentation | ||
|
||
Build and publish documentation with: | ||
|
||
```bash | ||
pnpm doc | ||
pnpm doc:publish | ||
pnpm typedoc | ||
pnpm typedoc:publish | ||
``` | ||
|
||
## Publishing | ||
### Publishing | ||
|
||
This package follows semantic versioning: | ||
|
||
1. Major Releases: Introduce significant changes, including breaking changes. | ||
2. Minor Releases: Add new features in a backwards-compatible manner. | ||
3. Patch Releases: Include backwards-compatible bug fixes. | ||
|
||
There are multiple inputs that go into deciding when to publish a release, usually driven by external factors like shipping something on the server side and wanting to test it. | ||
|
||
Currently, versions are manually updated in the `package.json` file of each package. | ||
This process involves determining the appropriate version number based on the changes being introduced (e.g., major, minor, patch). | ||
|
||
When the package is published to [npm](https://www.npmjs.com/package/@nucypher/taco), tags are used to indicate the environment or network compatibility. | ||
We use `devnet`, `testnet`, and `mainnet` help users understand which network the release is intended for. | ||
|
||
```bash | ||
npm dist-tag add @nucypher/[email protected] devnet | ||
``` | ||
|
||
Ideally, this would all be automated using Github actions, workflows, releases, and tags. | ||
|
||
### Version Control | ||
We use git and github to manage our version control. | ||
Where possible, please follow the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) specification. | ||
|
||
# External API | ||
This is the api that we expose to developers. | ||
It is defined in [`packages/taco/src/taco.ts`](https://github.com/nucypher/taco-web/blob/main/packages/taco/src/taco.ts) | ||
|
||
# Design and Architecture | ||
|
||
## Context Parameters | ||
When defining conditions for decryption, context parameters are specified to indicate what information is required. | ||
These parameters are placeholders that will be filled with actual data during the decryption process. | ||
|
||
The built in `:userAddress` parameter is defined in `packages/taco-auth/src/auth-provider.ts`. | ||
Custom context parameters can also be defined and they must always begin with `:`. | ||
|
||
[This test](https://github.com/nucypher/taco-web/blob/b689493a37bec0b168f80f43347818095c3dd5ce/packages/taco/test/taco.test.ts#L102) demonstrates the functionality: | ||
```typescript | ||
it('exposes requested parameters', async () => { | ||
const mockedDkg = fakeDkgFlow(FerveoVariant.precomputed, 0, 4, 4); | ||
const mockedDkgRitual = fakeDkgRitual(mockedDkg); | ||
const provider = fakeProvider(aliceSecretKeyBytes); | ||
const signer = fakeSigner(aliceSecretKeyBytes); | ||
const getFinalizedRitualSpy = mockGetActiveRitual(mockedDkgRitual); | ||
|
||
const customParamKey = ':nftId'; | ||
const ownsNFTWithCustomParams = new conditions.predefined.erc721.ERC721Ownership({ | ||
contractAddress: '0x1e988ba4692e52Bc50b375bcC8585b95c48AaD77', | ||
parameters: [customParamKey], | ||
chain: TEST_CHAIN_ID, | ||
}); | ||
|
||
const messageKit = await taco.encrypt( | ||
provider, | ||
domains.DEVNET, | ||
message, | ||
ownsNFTWithCustomParams, | ||
mockedDkg.ritualId, | ||
signer, | ||
); | ||
expect(getFinalizedRitualSpy).toHaveBeenCalled(); | ||
|
||
const requestedParameters = taco.conditions.context.ConditionContext.requestedContextParameters(messageKit); | ||
expect(requestedParameters).toEqual(new Set([customParamKey, USER_ADDRESS_PARAM_DEFAULT])); | ||
}); | ||
``` | ||
|
||
## Authentication Providers | ||
User authentication is handled by an `AuthProvider` defined in `packages/taco-auth/src`. | ||
Using authentication providers has several benefits: | ||
1. Abstraction: | ||
|
||
They abstract away the specific details of authentication, providing a unified interface for different authentication methods. | ||
|
||
2. Flexibility: | ||
|
||
The system can accommodate various types of authentication providers, making it extensible and adaptable to different authentication needs. | ||
|
||
3. User Context Management: | ||
|
||
They help manage user context, specifically the `:userAddress` parameter. This includes handling authentication tokens and ensuring that users are authenticated only once for multiple actions. | ||
|
||
Currently, [SIWE](https://docs.login.xyz/) (Sign-In With Ethereum, [EIP-4361](https://eips.ethereum.org/EIPS/eip-4361)) is the only authentication method implemented. | ||
EIP-712 has previously been supported but is now deprecated. | ||
|
||
[The below test](https://github.com/nucypher/taco-web/blob/b689493a37bec0b168f80f43347818095c3dd5ce/packages/taco/test/conditions/context.test.ts#L382C1-L429C6) demonstrates how a SIWE message can be reused for TACo authentication. | ||
This ensures that users don't have to sign multiple messages when logging into apps and decrypting TACo messages. | ||
theref marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
```typescript | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this test in a test file somewhere already? Can we link to to test files/line numbers instead? That would ensure that the test always remains fresh. Some for other spots above. |
||
it('supports reusing external eip4361', async () => { | ||
// Because we are reusing an existing SIWE auth message, we have to pass it as a custom parameter | ||
const authMessage = await makeAuthSignature(USER_ADDRESS_PARAM_DEFAULT); | ||
const customParams: Record<string, CustomContextParam> = { | ||
[USER_ADDRESS_PARAM_EXTERNAL_EIP4361]: authMessage as CustomContextParam, | ||
}; | ||
|
||
// Spying on the EIP4361 provider to make sure it's not called | ||
const eip4361Spy = vi.spyOn( | ||
EIP4361AuthProvider.prototype, | ||
'getOrCreateAuthSignature', | ||
); | ||
|
||
// Now, creating the condition context to run the actual test | ||
const conditionObj = { | ||
...testContractConditionObj, | ||
returnValueTest: { | ||
...testReturnValueTest, | ||
value: USER_ADDRESS_PARAM_EXTERNAL_EIP4361, | ||
}, | ||
}; | ||
const condition = new ContractCondition(conditionObj); | ||
const conditionExpr = new ConditionExpression(condition); | ||
|
||
// Make sure we remove the EIP4361 auth method from the auth providers first | ||
delete authProviders[EIP4361_AUTH_METHOD]; | ||
// Should throw an error if we don't pass the custom parameter | ||
expect( | ||
() => conditionExpr.buildContext( {}, authProviders) | ||
).toThrow( | ||
`No custom parameter for requested context parameter: ${USER_ADDRESS_PARAM_EXTERNAL_EIP4361}`, | ||
); | ||
|
||
// Remembering to pass in customParams here: | ||
const builtContext = conditionExpr.buildContext( | ||
customParams, | ||
authProviders, | ||
); | ||
const contextVars = await builtContext.toContextParameters(); | ||
expect(eip4361Spy).not.toHaveBeenCalled(); | ||
|
||
TODO: Update after implementing automated publishing. | ||
// Now, we expect that the auth signature will be available in the context variables | ||
const authSignature = contextVars[ | ||
USER_ADDRESS_PARAM_EXTERNAL_EIP4361 | ||
] as AuthSignature; | ||
expect(authSignature).toBeDefined(); | ||
await testEIP4361AuthSignature(authSignature); | ||
}); | ||
``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also the
:userAddressExternalEIP4361
context variable.It also uses an auth provider for generating the AuthSignature - https://github.com/nucypher/taco-web/blob/main/packages/taco-auth/src/providers/external-eip4361.ts#L6.
Side note: my goal is to update the API via #560 in the next sprint.