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

feat(solana)!: enable calling SetConfig without sending the transaction #310

Merged
merged 8 commits into from
Feb 24, 2025

Conversation

gustavogama-cll
Copy link
Contributor

@gustavogama-cll gustavogama-cll commented Feb 20, 2025

This PR enables clients to call SetConfig() without sending the instructions to the blockchain. Its purpose is to retrieve the instructions that would be sent -- to, for instance, use this instructions to build an mcms proposal. The instructions are returned in the TransactionResult object, in the RawData attribute (which used to be called RawTransaction and was renamed to reflect the fact it can hold other arbitrary types).

In order to "activate" this mode, clients must set the optional argument SkipTransaction. For instance:

// standard SetConfig, instructions are sent on-chain
result, err := NewConfigurer(client, auth, chainSelector).SetConfig(...)
assert.True(t, solana.IsSignature(result.Hash))


// SkipTransaction() added to SetConfig() -- instructions are NOT sent on-chain
result, err := NewConfigurer(client, auth, chainSelector, SkipTransaction()).SetConfig(...)
assert.Equal(t, result.Hash, "")
assert.GreaterOrEqual(t, len(result.RawData.([]solana.Instruction)), 4)

BREAKING_CHANGE: RawTransaction renamed to RawData

Copy link

changeset-bot bot commented Feb 20, 2025

🦋 Changeset detected

Latest commit: 0bfcacc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@smartcontractkit/mcms Minor

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

@gustavogama-cll gustavogama-cll force-pushed the enable-set-config-do-not-send-transaction branch from 419ea25 to 3bbdf4e Compare February 20, 2025 21:07
Copy link
Contributor

@graham-chainlink graham-chainlink left a comment

Choose a reason for hiding this comment

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

As you said, elegant api is the challenge here

@gustavogama-cll gustavogama-cll force-pushed the enable-set-config-do-not-send-transaction branch from d96b908 to 9551439 Compare February 21, 2025 22:50
@gustavogama-cll gustavogama-cll changed the title feat(solana)!: enable calling SetConfig without sending the transaction feat(solana): enable calling SetConfig without sending the transaction Feb 21, 2025
@gustavogama-cll gustavogama-cll changed the title feat(solana): enable calling SetConfig without sending the transaction feat(solana)!: enable calling SetConfig without sending the transaction Feb 21, 2025
@gustavogama-cll gustavogama-cll marked this pull request as ready for review February 22, 2025 01:54
@gustavogama-cll gustavogama-cll requested a review from a team as a code owner February 22, 2025 01:54
var err error
for i, instruction := range c.instructions {
signature, _, err = sendAndConfirmInstructions(ctx, client, auth,
[]solana.Instruction{instruction}, rpc.CommitmentConfirmed)
Copy link
Contributor

Choose a reason for hiding this comment

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

i assumed this wouldn work if we send the whole batch in one go instead of in a loop one at a time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure I follow; we're still sending+confirming one instruction at a time.

ecPablo
ecPablo previously approved these changes Feb 24, 2025
Copy link
Collaborator

@ecPablo ecPablo left a comment

Choose a reason for hiding this comment

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

LGTM!

This PR enables clients to call SetConfig() without sending the
instructions to the blockchain. Its purpose is to retrieve the instructions
that _would_ be sent -- to, for instance, use this instructions to
build an mcms proposal. The instructions are returned in the `TransactionResult` object, in the
`RawTransaction` attribute.

In order to "activate" this mode, clients must set leave the authority
private key as `nil` *and* specify the signer account in the
`instructionAuth` parameter. Existing clients (who still want to send
the instructions onchain) are also affected as we had to introduce a new
parameter to the Configurer constructor (`instructionAuth`).

Examples:
```go
// send instructions on-chain
result, err := NewConfigurer(client, auth, solana.PublicKey{}, chainSelector).SetConfig()
assert.True(t, solana.IsSignature(result.Hash))
assert.GreaterOrEqual(t, len(result.RawTransaction.([]solana.Instruction)), 4)
```

```go
// do NOT send instructions on-chain
result, err := NewConfigurer(client, auth, solana.PublicKey{}, chainSelector).SetConfig()
assert.Equal(t, result.Hash, "")
assert.GreaterOrEqual(t, len(result.RawTransaction.([]solana.Instruction)), 4)
```

!BREAKING CHANGE!
@gustavogama-cll gustavogama-cll added this pull request to the merge queue Feb 24, 2025
Merged via the queue into main with commit d5d9b16 Feb 24, 2025
8 checks passed
@gustavogama-cll gustavogama-cll deleted the enable-set-config-do-not-send-transaction branch February 24, 2025 20:19
github-merge-queue bot pushed a commit that referenced this pull request Feb 24, 2025
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
## @smartcontractkit/[email protected]

### Minor Changes

- [#297](#297)
[`2a4a443`](2a4a443)
Thanks [@graham-chainlink](https://github.com/graham-chainlink)! - fix:
make predecessors parameter optional on NewProposal and
NewTimelockProposal

- [#310](#310)
[`d5d9b16`](d5d9b16)
Thanks [@gustavogama-cll](https://github.com/gustavogama-cll)! - feat:
enable calling SetConfig without sending the transaction

- [#301](#301)
[`d8a156c`](d8a156c)
Thanks [@akhilchainani](https://github.com/akhilchainani)! - Expose
additional functions to help with proposal decoding

- [#313](#313)
[`4a4a5b5`](4a4a5b5)
Thanks [@ecPablo](https://github.com/ecPablo)! - Remove dependency of
timelock converter in solana with rpc.Client

### Patch Changes

- [#311](#311)
[`3808de9`](3808de9)
Thanks [@graham-chainlink](https://github.com/graham-chainlink)! -
Update to latest chainlink-ccip/chains/solana

Co-authored-by: app-token-issuer-engops[bot] <144731339+app-token-issuer-engops[bot]@users.noreply.github.com>
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