Skip to content

Commit a17b27f

Browse files
authored
Eth contract fix memory + new sdk + refactor (#257)
* Update sdk version * Update the contract according to the sdk changes - Change some memory modifiers to improve gas efficiency - Implement getValidTimePeriod() and remove old staleness logic - Update the tests * Update latest migration descriptions * Add version * Update Deploying.md * Add test to validate version of the contract * Add deploy commit hash * Rename the placeholder * Fix placeholder
1 parent 995c886 commit a17b27f

File tree

8 files changed

+89
-64
lines changed

8 files changed

+89
-64
lines changed

ethereum/Deploying.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ rm -rf build && npx truffle compile --all
1818
# Merge the network addresses into the artifacts, if some contracts are already deployed.
1919
npx apply-registry
2020

21+
# Set the deploy commit hash in the contract binary (used for debugging purposes)
22+
sed -i "s/dead0beaf0deb10700c0331700da5d00deadbead/$(git rev-parse HEAD)/g" build/contracts/*
23+
2124
# Perform the migration
2225
npx truffle migrate --network $MIGRATIONS_NETWORK
2326

@@ -60,6 +63,8 @@ const PythUpgradable = artifacts.require("PythUpgradable");
6063
const { upgradeProxy } = require("@openzeppelin/truffle-upgrades");
6164

6265
/**
66+
* Version <x.y.z>.
67+
*
6368
* Briefly describe the changelog here.
6469
*/
6570
module.exports = async function (deployer) {
@@ -86,6 +91,12 @@ make sure that your change to the contract won't cause any collision**. For exam
8691
Anything other than the operations above will probably cause a collision. Please refer to Open Zeppelin Upgradeable
8792
(documentations)[https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable] for more information.
8893

94+
## Versioning
95+
96+
We use [Semantic Versioning](https://semver.org/) for our releases. When upgrading the contract, update the npm package version using
97+
`npm version <new version number> --no-git-tag-version`. Also, modify the hard-coded value in `version()` method in
98+
[the `Pyth.sol` contract](./contracts/pyth/Pyth.sol) to the new version. Then, after your PR is merged in main, create a release like with tag `pyth-evm-contract-v<x.y.z>`. This will help developers to be able to track code changes easier.
99+
89100
# Testing
90101

91102
The [pyth-js][] repository contains an example with documentation and a code sample showing how to relay your own prices to a

ethereum/contracts/pyth/Pyth.sol

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
2424
setPyth2WormholeEmitter(pyth2WormholeEmitter);
2525
}
2626

27-
function updatePriceBatchFromVm(bytes memory encodedVm) private returns (PythInternalStructs.BatchPriceAttestation memory bpa) {
27+
function updatePriceBatchFromVm(bytes calldata encodedVm) private returns (PythInternalStructs.BatchPriceAttestation memory bpa) {
2828
(IWormhole.VM memory vm, bool valid, string memory reason) = wormhole().parseAndVerifyVM(encodedVm);
2929

3030
require(valid, reason);
@@ -211,34 +211,29 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
211211
}
212212

213213
function queryPriceFeed(bytes32 id) public view override returns (PythStructs.PriceFeed memory priceFeed){
214-
215214
// Look up the latest price info for the given ID
216215
PythInternalStructs.PriceInfo memory info = latestPriceInfo(id);
217216
require(info.priceFeed.id != 0, "no price feed found for the given price id");
218217

219-
// Check that there is not a significant difference between this chain's time
220-
// and the price publish time.
221-
if (info.priceFeed.status == PythStructs.PriceStatus.TRADING &&
222-
absDiff(block.timestamp, info.priceFeed.publishTime) > validTimePeriodSeconds()) {
223-
info.priceFeed.status = PythStructs.PriceStatus.UNKNOWN;
224-
// getLatestAvailablePrice* gets prevPrice when status is
225-
// unknown. So, now that status is being set to unknown,
226-
// we should move the current price to the previous
227-
// price to ensure getLatestAvailablePrice* works
228-
// as intended.
229-
info.priceFeed.prevPrice = info.priceFeed.price;
230-
info.priceFeed.prevConf = info.priceFeed.conf;
231-
info.priceFeed.prevPublishTime = info.priceFeed.publishTime;
232-
}
233-
234218
return info.priceFeed;
235219
}
236220

237-
function absDiff(uint x, uint y) private pure returns (uint) {
238-
if (x > y) {
239-
return x - y;
240-
} else {
241-
return y - x;
242-
}
221+
function priceFeedExists(bytes32 id) public override view returns (bool) {
222+
PythInternalStructs.PriceInfo memory info = latestPriceInfo(id);
223+
return (info.priceFeed.id != 0);
224+
}
225+
226+
function getValidTimePeriod() public override view returns (uint) {
227+
return validTimePeriodSeconds();
228+
}
229+
230+
function version() public pure returns (string memory) {
231+
return "0.1.0";
232+
}
233+
234+
function deployCommitHash() public pure returns (bytes20) {
235+
// This is a place holder for the commit hash and will be replaced
236+
// with the commit hash upon deployment.
237+
return hex"dead0beaf0deb10700c0331700da5d00deadbead";
243238
}
244239
}

ethereum/migrations/prod-receiver/8_pyth_update_interface_add_update_if_necessary.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,15 @@ const PythUpgradable = artifacts.require("PythUpgradable");
55
const { upgradeProxy } = require("@openzeppelin/truffle-upgrades");
66

77
/**
8+
* Version 0.1.0
9+
*
810
* This change:
911
* - Updates the interface, adds `updatePriceFeedsIfNecessary` that wraps
1012
* `updatePriceFeeds` and rejects if the price update is not necessary.
13+
* - Changes some memory modifiers to improve gas efficiency.
14+
* - Changes staleness logic to be included in the sdk and bring
15+
* more clarity to the existing code.
16+
* - Adds version to the contract (which is hard coded)
1117
*/
1218
module.exports = async function (deployer) {
1319
const proxy = await PythUpgradable.deployed();

ethereum/migrations/prod/7_pyth_update_interface_add_update_if_necessary.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,15 @@ const PythUpgradable = artifacts.require("PythUpgradable");
55
const { upgradeProxy } = require("@openzeppelin/truffle-upgrades");
66

77
/**
8+
* Version 0.1.0
9+
*
810
* This change:
911
* - Updates the interface, adds `updatePriceFeedsIfNecessary` that wraps
1012
* `updatePriceFeeds` and rejects if the price update is not necessary.
13+
* - Changes some memory modifiers to improve gas efficiency.
14+
* - Changes staleness logic to be included in the sdk and bring
15+
* more clarity to the existing code.
16+
* - Adds version to the contract (which is hard coded)
1117
*/
1218
module.exports = async function (deployer) {
1319
const proxy = await PythUpgradable.deployed();

ethereum/migrations/test/8_pyth_update_interface_add_update_if_necessary.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,15 @@ const PythUpgradable = artifacts.require("PythUpgradable");
55
const { upgradeProxy } = require("@openzeppelin/truffle-upgrades");
66

77
/**
8+
* Version 0.1.0
9+
*
810
* This change:
911
* - Updates the interface, adds `updatePriceFeedsIfNecessary` that wraps
1012
* `updatePriceFeeds` and rejects if the price update is not necessary.
13+
* - Changes some memory modifiers to improve gas efficiency.
14+
* - Changes staleness logic to be included in the sdk and bring
15+
* more clarity to the existing code.
16+
* - Adds version to the contract (which is hard coded)
1117
*/
1218
module.exports = async function (deployer) {
1319
const proxy = await PythUpgradable.deployed();

ethereum/package-lock.json

Lines changed: 11 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ethereum/package.json

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
{
2-
"name": "wormhole",
3-
"version": "1.0.0",
2+
"name": "@pythnetwork/pyth-evm-contract",
3+
"version": "0.1.0",
44
"description": "",
5-
"main": "networks.js",
65
"devDependencies": {
76
"@chainsafe/truffle-plugin-abigen": "0.0.1",
87
"@openzeppelin/cli": "^2.8.2",
@@ -30,7 +29,7 @@
3029
"dependencies": {
3130
"@openzeppelin/contracts": "^4.5.0",
3231
"@openzeppelin/contracts-upgradeable": "^4.5.2",
33-
"@pythnetwork/pyth-sdk-solidity": "^0.5.1",
32+
"@pythnetwork/pyth-sdk-solidity": "^0.5.2",
3433
"dotenv": "^10.0.0",
3534
"elliptic": "^6.5.2",
3635
"ganache-cli": "^6.12.1",

ethereum/test/pyth.js

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const PythStructs = artifacts.require("PythStructs");
66

77
const { deployProxy, upgradeProxy } = require("@openzeppelin/truffle-upgrades");
88
const { expectRevert, expectEvent, time } = require("@openzeppelin/test-helpers");
9-
const { assert } = require("chai");
9+
const { assert, expect } = require("chai");
1010

1111
// Use "WormholeReceiver" if you are testing with Wormhole Receiver
1212
const Wormhole = artifacts.require("Wormhole");
@@ -562,7 +562,7 @@ contract("Pyth", function () {
562562
);
563563
});
564564

565-
it("should show stale cached prices as unknown", async function () {
565+
it("should revert on getting stale current prices", async function () {
566566
let smallestTimestamp = 1;
567567
let rawBatch = generateRawBatchAttestation(
568568
smallestTimestamp,
@@ -575,15 +575,14 @@ contract("Pyth", function () {
575575
const price_id =
576576
"0x" +
577577
(255 - (i % 256)).toString(16).padStart(2, "0").repeat(32);
578-
let priceFeedResult = await this.pythProxy.queryPriceFeed(price_id);
579-
assert.equal(
580-
priceFeedResult.status.toString(),
581-
PythStructs.PriceStatus.UNKNOWN.toString()
578+
expectRevert(
579+
this.pythProxy.getCurrentPrice(price_id),
580+
"current price unavailable"
582581
);
583582
}
584583
});
585584

586-
it("should show cached prices too far into the future as unknown", async function () {
585+
it("should revert on getting current prices too far into the future as they are considered unknown", async function () {
587586
let largestTimestamp = 4294967295;
588587
let rawBatch = generateRawBatchAttestation(
589588
largestTimestamp - 5,
@@ -596,12 +595,11 @@ contract("Pyth", function () {
596595
const price_id =
597596
"0x" +
598597
(255 - (i % 256)).toString(16).padStart(2, "0").repeat(32);
599-
let priceFeedResult = await this.pythProxy.queryPriceFeed(price_id);
600-
assert.equal(
601-
priceFeedResult.status.toString(),
602-
PythStructs.PriceStatus.UNKNOWN.toString()
603-
);
604-
}
598+
expectRevert(
599+
this.pythProxy.getCurrentPrice(price_id),
600+
"current price unavailable"
601+
);
602+
}
605603
});
606604

607605
it("changing validity time works", async function() {
@@ -622,11 +620,9 @@ contract("Pyth", function () {
622620
const price_id =
623621
"0x" +
624622
(255 - (i % 256)).toString(16).padStart(2, "0").repeat(32);
625-
let priceFeedResult = await this.pythProxy.queryPriceFeed(price_id);
626-
assert.equal(
627-
priceFeedResult.status.toString(),
628-
PythStructs.PriceStatus.TRADING.toString()
629-
);
623+
624+
// Expect getCurrentPrice to work (not revert)
625+
await this.pythProxy.getCurrentPrice(price_id);
630626
}
631627

632628
// One minute passes
@@ -637,10 +633,10 @@ contract("Pyth", function () {
637633
const price_id =
638634
"0x" +
639635
(255 - (i % 256)).toString(16).padStart(2, "0").repeat(32);
640-
let priceFeedResult = await this.pythProxy.queryPriceFeed(price_id);
641-
assert.equal(
642-
priceFeedResult.status.toString(),
643-
PythStructs.PriceStatus.UNKNOWN.toString()
636+
637+
expectRevert(
638+
this.pythProxy.getCurrentPrice(price_id),
639+
"current price unavailable"
644640
);
645641
}
646642

@@ -653,10 +649,9 @@ contract("Pyth", function () {
653649
"0x" +
654650
(255 - (i % 256)).toString(16).padStart(2, "0").repeat(32);
655651
let priceFeedResult = await this.pythProxy.queryPriceFeed(price_id);
656-
assert.equal(
657-
priceFeedResult.status.toString(),
658-
PythStructs.PriceStatus.TRADING.toString()
659-
);
652+
653+
// Expect getCurrentPrice to work (not revert)
654+
await this.pythProxy.getCurrentPrice(price_id);
660655
}
661656
});
662657

@@ -724,6 +719,13 @@ contract("Pyth", function () {
724719
"invalid data source chain/emitter ID"
725720
);
726721
});
722+
723+
it("Make sure version is the npm package version", async function () {
724+
const contractVersion = await this.pythProxy.version();
725+
const { version } = require('../package.json');
726+
727+
expect(contractVersion).equal(version);
728+
});
727729
});
728730

729731
const signAndEncodeVM = async function (

0 commit comments

Comments
 (0)