-
Notifications
You must be signed in to change notification settings - Fork 164
Simple mina-signer integration test app #2646
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
base: main
Are you sure you want to change the base?
Conversation
…ansaction signed by mina-signer is not rejected from live node. At this moment app just verifies if tranasction is safely inserted in the pool
bleepbloopsify
left a comment
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.
Your USAGE_INFO is a bit inconsistent, as is the bin and name of the package.json
since you have references to both test-signer and mina-test-signer, I can see that you probably changed the name mid-way, and didn't go through and fix it properly
We should pick one or the other in this case, that way its not confusing in usage-info or the commander config
The README also has a bunch of inconsistencies
| - Treat private keys in plain text with care. Prefer environment variables or a | ||
| secure secrets manager for real deployments. | ||
| - The example keys under `key/` are for local testing only; they are publicly | ||
| known and should never hold funds. |
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.
Is there a private key in plaintext somewhere in this repo?
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.
No, it's just the one we used for dry run network when testing hf. Randomly generated.
| ## Customisation Tips | ||
|
|
||
| - Update `CONFIG.DEFAULT_GRAPHQL_URL` in `config.js` to point at your daemon or | ||
| a hosted GraphQL endpoint. | ||
| - Tweak `CONFIG.MINA_UNITS.DEFAULT_AMOUNT_MULTIPLIER` and | ||
| `DEFAULT_FEE_MULTIPLIER` to adjust the default transaction values. | ||
| - Extend `GraphQLClient` with additional queries (e.g. account state, balances) | ||
| if you need richer diagnostics. |
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.
I believe this section isn't required either, since we've already customized with CLI args
| 2. `payment-service.js` derives the sender public key, composes a payment | ||
| payload, and signs it with `mina-signer`. |
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.
I don't think this file exists?
| const response = await fetch(this.url, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ operationName: null, query, variables: {} }), |
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.
Do we need to pass null under operationName here?
Similar question with the empty variables block
| "dependencies": { | ||
| "commander": "^14.0.2", | ||
| "json-to-graphql-query": "^2.3.0", | ||
| "mina-signer": "^1.7.0" |
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.
should this use the local mina-signer instead?
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.
fixing it
9c4a146 to
9bc9958
Compare
As part of splitting of MinaProtocol/mina#17944, adding mina-test-signer app which is supposed to test integration between mina-signer and daemon node..
As part of automated effort, adding mina-signer test app which sends transaction and validate it is included in the pool. This is part of runbook: https://www.notion.so/o1labs/A16-Validate-build-263e79b1f91081e9a07ed6970a3dbc07. Section Build 1.a.
Example usage of app: