Fixes: Sui chain & ctf provider fixes #292
Conversation
🦋 Changeset detectedLatest commit: ef65c94 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
👋 stackman27, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
stackman27
force-pushed
the
sish/sui-fixes
branch
from
73b0df3 to
97817af
Compare
There was a problem hiding this comment.
any reason why we want to hardcode the faucet port? It was originally introduced due to test flakiness where the same port maybe already taken, using freeport allowed us to allocate a port that is not used.
Alterntive, if you want to use that port, we could expose on the config to allow user to specify the port they need instead.
chain/sui/signer.go
Outdated
|
|
||
| // PrivateKey returns the underlying ed25519.PrivateKey (64 bytes = seed||pubkey) | ||
| // from a SuiSigner created in this package. | ||
| func PrivateKey(s SuiSigner) (ed25519.PrivateKey, error) { |
There was a problem hiding this comment.
Is this something we want to make public? This means anyone with access to Signer can retrieve the private key which increases the surface area for it to be leaked.
There was a problem hiding this comment.
hmm any opposition in just retrieving the signer, i need it in the format that blockvision returns
There was a problem hiding this comment.
hmm but seems like the signer has the PriKey as exported , which may be an issue if we return the signer, any alternative like performing the signing here instead of passing the private key out?
Co-authored-by: Graham Goh <[email protected]>
Co-authored-by: Graham Goh <[email protected]>
chain/sui/signer.go
Outdated
| } | ||
|
|
||
| // GetSigner returns the signer in blockVision signer.Signer interface format | ||
| func GetSigner(s SuiSigner) (*signer.Signer, error) { |
There was a problem hiding this comment.
just raising that returning the signer here exposes the private key which is not something we should do.
There was a problem hiding this comment.
we should look at why we need to have the signer and what we do with it, then see if we can push that behaviour over to the signer instead
|
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## [email protected] ### Patch Changes - [#292](#292) [`42dc440`](42dc440) Thanks [@stackman27](https://github.com/stackman27)! - fixes for sui provider --------- Co-authored-by: app-token-issuer-engops[bot] <144731339+app-token-issuer-engops[bot]@users.noreply.github.com>
No description provided.