Skip to content

Conversation

@drozdziak1
Copy link
Contributor

This commit resolves a runtime problem in migrate() which occurred on devnet, as
well as invalid mutability for the old config and misuse of the
checked_sub() method.

The runtime problem came from a non-zero balance existing on
not-yet-used PDA. Effectively, this means permissionless
initialization of PDAs on Solana. Anyone can send lamports to any
account on Solana (existing or not), and it doesn't take much funds to
keep an empty account exempt. While it is possible to recover from it,
this case is relatively rare and takes a lot of code to
automate. Instead, we settle for a dumb bump of the config PDA
seed (see config.rs).

This measure is best used sparingly, as it imposes the same static
seed on all SOL networks. This will cause headaches if the attester
tooling grows a third-party user base.

This commit resolves a runtime problem in migrate() which occurred on devnet, as
well as invalid mutability for the old config and misuse of the
checked_sub() method.

The runtime problem came from a non-zero balance existing on
not-yet-used PDA. Effectively, this means permissionless
initialization of PDAs on Solana. Anyone can send lamports to any
account on Solana (existing or not), and it doesn't take much funds to
keep an empty account exempt. While it is possible to recover from it,
this case is relatively rare and takes a lot of code to
automate. Instead, we settle for a dumb bump of the config PDA
seed (see config.rs).

This measure is best used sparingly, as it imposes the same static
seed on all SOL networks. This will cause headaches if the attester
tooling grows a third-party user base.
Copy link
Collaborator

@ali-behjati ali-behjati left a comment

Choose a reason for hiding this comment

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

Nice! And thanks for your detailed explanation Stan 🙇

Nice catch on wrong mutability of old config. It is still a bit strange that how tests passed with the old config being immutable 🤔 does it mean that contract does not panic on such a change?

btw, I think wherever we go, we manage the attestation contract ourself and let only attesters to be permissionless. So there's no problem to change the version.

@drozdziak1 drozdziak1 merged commit 130c052 into main Aug 19, 2022
@drozdziak1 drozdziak1 deleted the drozdziak1/pyth2wormhole-cfg-migration-fix branch August 19, 2022 15:45
@drozdziak1
Copy link
Contributor Author

On a closing note: I think the version still affects permissionless attesters because there's still need to compute the config PDA address from seeds. For that reason, they need the p2w crates to be up-to-date with what the contract expects.

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