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

fix: update path in Solana readme #1146

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

lzdane
Copy link

@lzdane lzdane commented Jan 5, 2025

No description provided.

@ryandgoulding ryandgoulding requested a review from nazreen January 6, 2025 01:29

```
endpoint: <ENDPOINT_PROGRAM_ID>
oft: <OFT_PROGRAM_ID>
```

Copy the OFT's programId and go into [lib.rs](./programs/oft/src/lib.rs). Note the following snippet:
Copy the OFT's programId and go into [lib.rs](./programs/oft/src/lib.rs) (located inside this project under oft-solana/programs/oft/src/lib.rs). Note the following snippet:
Copy link
Contributor

Choose a reason for hiding this comment

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

@lzdane I don't think there's a need for the note in parentheses, the link already links to the right file. Devs can click on it. It's normal assumption that a filename with a link in a README links to a file in the same repo.

@@ -210,7 +210,7 @@ pnpm hardhat lz:oft:solana:create --eid 40168 --program-id <PROGRAM_ID>

:warning: Use `--additional-minters` flag to add a CSV of additional minter addresses to the Mint Authority Multisig. If you do not want to, you must specify `--only-oft-store true`.

:information_source: You can also specify `--amount <AMOUNT>` to have the OFT minted to your deployer address upon token creation.
:information_source: You can also specify `--amount <AMOUNT>` to have the OFT minted to your deployer address upon token creation. Remember this is in local decimals, so with local decimals of 9, you would need to pass in `--amount1000000000` to mint 1 OFT.
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for adding this!

btw there's a space missing

amount1000000000

Copy link
Contributor

@DanL0 DanL0 left a comment

Choose a reason for hiding this comment

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

I agree with @nazreen 's comments. If you address his comments this looks good to me.

Also please fix linting: https://github.com/LayerZero-Labs/devtools/actions/runs/12632527710/job/35196385732

We can't merge PR with failing CI checks.

@nazreen
Copy link
Contributor

nazreen commented Jan 13, 2025

@lzdane if you resolve the 2 change requests and fix linting as @DanL0 pointed out, we can get this approved and merged!

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