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

[ECO-2167] Rename all instances of _PK to _PRIVATE_KEY in favor of being more explicit about the key type #235

Merged
merged 4 commits into from
Sep 9, 2024

Conversation

xbtmatt
Copy link
Collaborator

@xbtmatt xbtmatt commented Sep 4, 2024

Description

Rename all instances of <SOME_ACCOUNT_NAME>_PK to end with _PRIVATE_KEY instead.

This is to avoid confusion between public keys and secret keys, making it slightly more explicit.

Testing

CI in this PR.

Checklist

  • Did you update relevant documentation?
  • Did you add tests to cover new code or a fixed issue?
  • Did you update the changelog?
  • Did you check all checkboxes from the linked Linear task?

@xbtmatt xbtmatt requested review from alnoki and CRBl69 September 4, 2024 21:54
Copy link

vercel bot commented Sep 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
emojicoin-dot-fun ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 9, 2024 8:02am
emojicoin-dot-fun-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 9, 2024 8:02am

Copy link
Collaborator

@CRBl69 CRBl69 left a comment

Choose a reason for hiding this comment

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

It seems that in src/typescript/sdk/tests/utils/helpers.ts, there are a couple of PK and pk left:

sdk/tests/e2e/publish.test.ts
20:      pk: publisher.privateKey,

sdk/tests/utils/helpers.ts
9:export const PK_PATH = path.resolve(path.join(TS_UNIT_TEST_DIR, ".tmp", ".pk"));
27:  const pk = fs.readFileSync(PK_PATH).toString();
29:    privateKey: new Ed25519PrivateKey(Hex.fromHexString(pk).toUint8Array()),

sdk/tests/pre-test.js
26:  const pk = await getTestPublisherPrivateKey();
27:  if (!pk) {
30:  fs.writeFileSync(PK_PATH, pk);
31:  const publishResult = JSON.stringify(await publishForTest(pk), null, 2);

sdk/tests/utils/publish.ts
18:  pk: PrivateKey;
24:  const { pk, network, namedAddresses, packageDirRelativeToRoot: packageDirRelative } = args;
42:  const pkString = new Hex(pk.toUint8Array()).toStringWithoutPrefix();
53:    ...["--private-key", pkString],
109:export async function publishForTest(pk: string): Promise<PublishPackageResult> {
112:    privateKey: new Ed25519PrivateKey(Hex.fromHexString(pk).toUint8Array()),
133:    pk: publisher.privateKey,

matt added 2 commits September 6, 2024 15:14
@xbtmatt
Copy link
Collaborator Author

xbtmatt commented Sep 6, 2024

It seems that in src/typescript/sdk/tests/utils/helpers.ts, there are a couple of PK and pk left:

sdk/tests/e2e/publish.test.ts
20:      pk: publisher.privateKey,

sdk/tests/utils/helpers.ts
9:export const PK_PATH = path.resolve(path.join(TS_UNIT_TEST_DIR, ".tmp", ".pk"));
27:  const pk = fs.readFileSync(PK_PATH).toString();
29:    privateKey: new Ed25519PrivateKey(Hex.fromHexString(pk).toUint8Array()),

sdk/tests/pre-test.js
26:  const pk = await getTestPublisherPrivateKey();
27:  if (!pk) {
30:  fs.writeFileSync(PK_PATH, pk);
31:  const publishResult = JSON.stringify(await publishForTest(pk), null, 2);

sdk/tests/utils/publish.ts
18:  pk: PrivateKey;
24:  const { pk, network, namedAddresses, packageDirRelativeToRoot: packageDirRelative } = args;
42:  const pkString = new Hex(pk.toUint8Array()).toStringWithoutPrefix();
53:    ...["--private-key", pkString],
109:export async function publishForTest(pk: string): Promise<PublishPackageResult> {
112:    privateKey: new Ed25519PrivateKey(Hex.fromHexString(pk).toUint8Array()),
133:    pk: publisher.privateKey,

Addressed here: d7f8f75

Thanks for catching this btw

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