Fixes #380
Fixes #380
Conversation
| @@ -343,20 +345,25 @@ func (m *OCRv2) proposeConfig(ocConfig contracts.OffChainAggregatorV2Config) err | |||
| } | |||
| // set one payee for all | |||
| instr := make([]solana.Instruction, 0) | |||
| // TODO: get associated addr | |||
gauntlet/packages/gauntlet-solana-contracts/src/commands/contracts/token/transfer.ts
Outdated
Show resolved
Hide resolved
gauntlet/packages/gauntlet-solana-contracts/src/commands/contracts/ocr2/initialize.ts
Outdated
Show resolved
Hide resolved
| @@ -86,7 +86,7 @@ pub mod ocr2 { | |||
| ) -> Result<()> { | |||
| let mut proposal = ctx.accounts.proposal.load_init()?; | |||
|
|
|||
| proposal.version = 1; | |||
| proposal.version = STATE_VERSION; | |||
There was a problem hiding this comment.
Could these two versions diverge?
If yes, let's use a specific proposal version constant.
There was a problem hiding this comment.
I think they should be in sync, an older proposal shouldn't be applied to a newer state because the layout expectations might be different
| payee.to_account_info() | ||
| } else { | ||
| // then the token account must be closed | ||
| token_receiver.to_account_info() |
There was a problem hiding this comment.
Shouldn't this happen only on close?
In other situations it could be more efficient to skip the oracle and keep the funds in the contract (on pay_oracles, accept_proposal & set_billing).
The way it's currently designed this will require setting this additional token_receiver account when calling these non-close methods, which might be confusing for contract operators.
There was a problem hiding this comment.
You can't skip the oracle because then the oracles aren't fully paid out so accept_proposal/set_billing won't succeed. Are you proposing to just ignore the oracle and then have the operator manually withdraw funds to resolve? That could work, but it would be easy to miss if one of the nodes didn't get paid out.
I think I set the token_receiver to whoever executes the call currently so there's no extra parameter.
There was a problem hiding this comment.
I was suggesting that the owner probably doesn't expect to collect some dust every time they set/accept_proposal or set_billing, so it might go unnoticed.
I think I'd prefer these operations to skip the invalid payee, keep the funds in the contract and then all the remaining funds are transferred out either on close (rare occasion) or potentially an owner can withdraw on behalf of an invalid oracle (new fn).
Feels cleaner to do it that way, but maybe it's just my preference. Wdyt?
|
|
||
| const CHAINLINK_PROGRAM_ID = "HEvSKofvBgfaexv23kMabbYqxasxU3mQ4ibBMEmJWHny"; | ||
|
|
||
| describe('hello-world', () => { | ||
| const provider = anchor.Provider.env(); | ||
| describe("hello-world", () => { |
There was a problem hiding this comment.
I've noticed many style changes in this PR but looks like we don't have prettier configured in this repository. I suggest we use the one from StarkNet integration and we reformat the codebase one more time: https://github.com/smartcontractkit/chainlink-starknet/blob/develop/.prettierrc.json
There was a problem hiding this comment.
Let's do that in a follow-up PR since the diff is already quite big
| // await store.account.transmissions.createInstruction(transmissions, 8+192+8096*24), | ||
| // createFeed, |
There was a problem hiding this comment.
I hoped to create these in a single transaction but had some issues getting it to work.
There was a problem hiding this comment.
Huge effort and nice improvements. Great work, @archseer!
I've reviewed to the best of my ability and it looks solid.
Except the CI e2e test failing, there is this discussion on oracle payments we might still consider, otherwise looks good.
Best reviewed commit-by-commit, I'm still cleaning up the branch.