Skip to content

Add verifier to pyth evm contract #2784

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions target_chains/ethereum/contracts/contracts/pyth/Pyth.sol
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ abstract contract Pyth is
) {
(
uint offset,
UpdateType updateType
) = extractUpdateTypeFromAccumulatorHeader(updateData[i]);
UpdateType updateType,

) = extractAccumulatorHeaderDetails(updateData[i]);
if (updateType != UpdateType.WormholeMerkle) {
revert PythErrors.InvalidUpdateData();
}
Expand Down Expand Up @@ -135,8 +136,9 @@ abstract contract Pyth is
) {
(
uint offset,
UpdateType updateType
) = extractUpdateTypeFromAccumulatorHeader(updateData[0]);
UpdateType updateType,

) = extractAccumulatorHeaderDetails(updateData[0]);
if (updateType != UpdateType.WormholeMerkle) {
revert PythErrors.InvalidUpdateData();
}
Expand Down Expand Up @@ -273,9 +275,10 @@ abstract contract Pyth is
}

uint offset;
Signer signer;
{
UpdateType updateType;
(offset, updateType) = extractUpdateTypeFromAccumulatorHeader(
(offset, updateType, signer) = extractAccumulatorHeaderDetails(
singleUpdateData
);

Expand All @@ -293,10 +296,7 @@ abstract contract Pyth is
merkleData.numUpdates,
encoded,
merkleData.slot
) = extractWormholeMerkleHeaderDigestAndNumUpdatesAndEncodedAndSlotFromAccumulatorUpdate(
singleUpdateData,
offset
);
) = extractWormholeMerkleHeader(singleUpdateData, signer, offset);

// Process each update within the Merkle proof
for (uint j = 0; j < merkleData.numUpdates; j++) {
Expand Down Expand Up @@ -435,12 +435,13 @@ abstract contract Pyth is
)
{
UpdateType updateType;
Signer signer;
uint offset;
bytes20 digest;
uint8 numUpdates;
bytes calldata encoded;
// Extract and validate the header for start data
(offset, updateType) = extractUpdateTypeFromAccumulatorHeader(
(offset, updateType, signer) = extractAccumulatorHeaderDetails(
updateData
);

Expand All @@ -455,10 +456,7 @@ abstract contract Pyth is
encoded,
// slot ignored

) = extractWormholeMerkleHeaderDigestAndNumUpdatesAndEncodedAndSlotFromAccumulatorUpdate(
updateData,
offset
);
) = extractWormholeMerkleHeader(updateData, signer, offset);

// Add additional validation before extracting TWAP price info
if (offset >= updateData.length) {
Expand Down
65 changes: 45 additions & 20 deletions target_chains/ethereum/contracts/contracts/pyth/PythAccumulator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,44 @@ abstract contract PythAccumulator is PythGetters, PythSetters, AbstractPyth {
TwapPriceFeed
}

enum Signer {
Wormhole,
Verifier
}

// This method is also used by batch attestation but moved here
// as the batch attestation will deprecate soon.
function parseAndVerifyPythVM(
bytes calldata encodedVm
bytes calldata encodedVm,
Signer signer
) internal view returns (IWormhole.VM memory vm) {
{
bool valid;
bool valid;
if (signer == Signer.Wormhole) {
(vm, valid, ) = wormhole().parseAndVerifyVM(encodedVm);
if (!valid) revert PythErrors.InvalidWormholeVaa();
} else if (signer == Signer.Verifier) {
if (address(verifier()) == address(0))
revert PythErrors.InvalidUpdateData();
(vm, valid, ) = verifier().parseAndVerifyVM(encodedVm);
} else {
revert PythErrors.InvalidSigner();
}

if (!valid) {
revert PythErrors.InvalidUpdateData();
}

if (!isValidDataSource(vm.emitterChainId, vm.emitterAddress))
if (!isValidDataSource(vm.emitterChainId, vm.emitterAddress)) {
revert PythErrors.InvalidUpdateDataSource();
}
}

function extractUpdateTypeFromAccumulatorHeader(
function extractAccumulatorHeaderDetails(
bytes calldata accumulatorUpdate
) internal pure returns (uint offset, UpdateType updateType) {
)
internal
pure
returns (uint offset, UpdateType updateType, Signer signer)
{
unchecked {
offset = 0;

Expand Down Expand Up @@ -90,13 +110,14 @@ abstract contract PythAccumulator is PythGetters, PythSetters, AbstractPyth {
);
offset += 1;

// We use another offset for the trailing header and in the end add the
// offset by trailingHeaderSize to skip the future headers.
//
// An example would be like this:
// uint trailingHeaderOffset = offset
// uint x = UnsafeBytesLib.ToUint8(accumulatorUpdate, trailingHeaderOffset)
// trailingHeaderOffset += 1
uint trailingHeaderOffset = offset;
Copy link
Collaborator

Choose a reason for hiding this comment

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

shall we change MINIMUM_ALLOWED_MINOR_VERSION?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why do we need to change that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

it is a metadata to tell us on which version we are right now and reject messages with older minor version. if we don't check it and this gets upgraded before Hermes there might be problems.

signer = Signer(
Copy link
Collaborator

Choose a reason for hiding this comment

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

i've lost my solidity knowledge here, what would happen if this byte does not fit in Signer (which accepts only 0 and 1). let's check it if it's an undefined behaviour.

UnsafeCalldataBytesLib.toUint8(
accumulatorUpdate,
trailingHeaderOffset
)
);
trailingHeaderOffset += 1;

offset += trailingHeaderSize;
}
Expand All @@ -112,8 +133,9 @@ abstract contract PythAccumulator is PythGetters, PythSetters, AbstractPyth {
}
}

function extractWormholeMerkleHeaderDigestAndNumUpdatesAndEncodedAndSlotFromAccumulatorUpdate(
function extractWormholeMerkleHeader(
bytes calldata accumulatorUpdate,
Signer signer,
uint encodedOffset
)
internal
Expand Down Expand Up @@ -148,7 +170,8 @@ abstract contract PythAccumulator is PythGetters, PythSetters, AbstractPyth {
encoded,
offset,
whProofSize
)
),
signer
);
offset += whProofSize;

Expand All @@ -171,7 +194,7 @@ abstract contract PythAccumulator is PythGetters, PythSetters, AbstractPyth {
UpdateType updateType = UpdateType(
UnsafeBytesLib.toUint8(encodedPayload, payloadOffset)
);
++payloadOffset;
payloadOffset += 1;

if (updateType != UpdateType.WormholeMerkle)
revert PythErrors.InvalidUpdateData();
Expand Down Expand Up @@ -460,8 +483,9 @@ abstract contract PythAccumulator is PythGetters, PythSetters, AbstractPyth {
) internal returns (uint8 numUpdates) {
(
uint encodedOffset,
UpdateType updateType
) = extractUpdateTypeFromAccumulatorHeader(accumulatorUpdate);
UpdateType updateType,
Signer signer
) = extractAccumulatorHeaderDetails(accumulatorUpdate);

if (updateType != UpdateType.WormholeMerkle) {
revert PythErrors.InvalidUpdateData();
Expand All @@ -477,8 +501,9 @@ abstract contract PythAccumulator is PythGetters, PythSetters, AbstractPyth {
numUpdates,
encoded,
slot
) = extractWormholeMerkleHeaderDigestAndNumUpdatesAndEncodedAndSlotFromAccumulatorUpdate(
) = extractWormholeMerkleHeader(
accumulatorUpdate,
signer,
encodedOffset
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,8 @@ contract PythGetters is PythState {
function transactionFeeInWei() public view returns (uint) {
return _state.transactionFeeInWei;
}

function verifier() public view returns (IWormhole) {
return IWormhole(_state.verifier);
}
}
14 changes: 14 additions & 0 deletions target_chains/ethereum/contracts/contracts/pyth/PythGovernance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ abstract contract PythGovernance is
);
event TransactionFeeSet(uint oldFee, uint newFee);
event FeeWithdrawn(address targetAddress, uint fee);
event VerifierAddressSet(
address oldVerifierAddress,
address newVerifierAddress
);

function verifyGovernanceVM(
bytes memory encodedVM
Expand Down Expand Up @@ -105,6 +109,8 @@ abstract contract PythGovernance is
setTransactionFee(parseSetTransactionFeePayload(gi.payload));
} else if (gi.action == GovernanceAction.WithdrawFee) {
withdrawFee(parseWithdrawFeePayload(gi.payload));
} else if (gi.action == GovernanceAction.SetVerifierAddress) {
setVerifierAddress(parseSetVerifierAddressPayload(gi.payload));
} else {
revert PythErrors.InvalidGovernanceMessage();
}
Expand Down Expand Up @@ -270,4 +276,12 @@ abstract contract PythGovernance is

emit FeeWithdrawn(payload.targetAddress, payload.fee);
}

function setVerifierAddress(
SetVerifierAddressPayload memory payload
) internal {
address oldVerifierAddress = address(verifier());
setVerifier(payload.newVerifierAddress);
emit VerifierAddressSet(oldVerifierAddress, address(verifier()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ contract PythGovernanceInstructions {
SetWormholeAddress, // 6
SetFeeInToken, // 7 - No-op for EVM chains
SetTransactionFee, // 8
WithdrawFee // 9
WithdrawFee, // 9
SetVerifierAddress // 10
Copy link
Collaborator

Choose a reason for hiding this comment

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

note for the future: we need to support it in governance sdk in TS

}

struct GovernanceInstruction {
Expand Down Expand Up @@ -90,6 +91,10 @@ contract PythGovernanceInstructions {
uint256 fee;
}

struct SetVerifierAddressPayload {
address newVerifierAddress;
}

/// @dev Parse a GovernanceInstruction
function parseGovernanceInstruction(
bytes memory encodedInstruction
Expand Down Expand Up @@ -272,4 +277,17 @@ contract PythGovernanceInstructions {
if (encodedPayload.length != index)
revert PythErrors.InvalidGovernanceMessage();
}

/// @dev Parse a UpdateVerifierAddressPayload (action 10) with minimal validation
function parseSetVerifierAddressPayload(
bytes memory encodedPayload
) public pure returns (SetVerifierAddressPayload memory sv) {
uint index = 0;

sv.newVerifierAddress = address(encodedPayload.toAddress(index));
index += 20;

if (encodedPayload.length != index)
revert PythErrors.InvalidGovernanceMessage();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,8 @@ contract PythSetters is PythState, IPythEvents {
function setTransactionFeeInWei(uint fee) internal {
_state.transactionFeeInWei = fee;
}

function setVerifier(address vf) internal {
_state.verifier = payable(vf);
}
}
2 changes: 2 additions & 0 deletions target_chains/ethereum/contracts/contracts/pyth/PythState.sol
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ contract PythStorage {
mapping(bytes32 => PythInternalStructs.PriceInfo) latestPriceInfo;
// Fee charged per transaction, in addition to per-update fees
uint transactionFeeInWei;
// Verifier address for verifying VAA signatures
address verifier;
}
}

Expand Down
2 changes: 1 addition & 1 deletion target_chains/ethereum/contracts/forge-test/Executor.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import "../contracts/executor/ExecutorUpgradable.sol";
import "./utils/WormholeTestUtils.t.sol";
import "./utils/InvalidMagic.t.sol";

contract ExecutorTest is Test, WormholeTestUtils {
contract ExecutorTest is Test, AbstractWormholeTestUtils {
Wormhole public wormhole;
ExecutorUpgradable public executor;
ExecutorUpgradable public executor2;
Expand Down
10 changes: 6 additions & 4 deletions target_chains/ethereum/contracts/forge-test/GasBenchmark.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import "@pythnetwork/pyth-sdk-solidity/PythStructs.sol";
import "./utils/WormholeTestUtils.t.sol";
import "./utils/PythTestUtils.t.sol";

contract GasBenchmark is Test, WormholeTestUtils, PythTestUtils {
contract GasBenchmark is Test, PythTestUtils {
// 19, current mainnet number of guardians, is used to have gas estimates
// close to our mainnet transactions.
uint8 constant NUM_GUARDIANS = 19;
Expand Down Expand Up @@ -53,9 +53,11 @@ contract GasBenchmark is Test, WormholeTestUtils, PythTestUtils {
uint randomSeed;

function setUp() public {
address wormholeAddr = setUpWormholeReceiver(NUM_GUARDIANS);
wormhole = IWormhole(wormholeAddr);
pyth = IPyth(setUpPyth(wormholeAddr));
WormholeTestUtils wormholeTestUtils = new WormholeTestUtils(
NUM_GUARDIANS
);
wormhole = IWormhole(wormholeTestUtils.getWormholeReceiverAddr());
pyth = IPyth(setUpPyth(wormholeTestUtils));

priceIds = new bytes32[](NUM_PRICES);
priceIds[0] = bytes32(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import "../contracts/pulse/SchedulerUpgradeable.sol";
import "@pythnetwork/pulse-sdk-solidity/SchedulerErrors.sol";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ contract PythAaveTest is PythWormholeMerkleAccumulatorTest {
uint constant VALID_TIME_PERIOD_SECS = 60;

function setUp() public override {
pyth = IPyth(setUpPyth(setUpWormholeReceiver(1)));
pyth = IPyth(setUpPyth(new WormholeTestUtils(1)));
assets = new address[](NUM_PRICE_FEEDS);
PriceFeedMessage[]
memory priceFeedMessages = generateRandomBoundedPriceFeedMessage(
Expand Down
Loading
Loading