Release - 1.2.8 #3510

ryanio · 2020-05-08T18:45:21Z

Description

This PR introduces web3.js version 1.2.8 beginning with 1.2.8-rc.0.

Features

Alongside some Typescript improvements and patches for the Websocket Provider reconnection logic introduced in 1.2.7, this release contains two larger changes:

Ethers ABI Coder has received a full version increment, from 4.0.0-beta.3 to 5.0.0-beta.153. Since 2018, Web3 has relied on @ricmoo's great Ethers project to manage encoding and decoding data passed between Web3 and the EVM. This update should allow Web3 to absorb the latest work at Ethers going forward.
The ENS module now supports ENS's contenthash feature (EIP 1577), which allows you to associate an IPFS or Swarm hash with an ENS domain. There's a nice introduction to content hashes in this ENS medium post. Docs for the newly added methods can be found here.

Added

Introduce review and release guidelines. (Introduce Review and Release Guidelines #3460)
Add EIP-1193 compatible provider to AbstractProvider interface. (Add EIP-1193 compatible provider to AbstractProvider interface #3499)
Add Typescript definitions for contract methods and call. (web3-eth-contract: add typescript definitions for methods and call #3454)
Add support for ENS contenthash methods ( web3.eth.ens.getContent seems to do not work properly #3392, Unexpected behavior when calling web3.eth.ens.getContenthash #2782)

Changed

Change CI provider from Travis to Github Actions. (Upgrade CI Provider from Travis to GH Actions #3468)
Update web3-eth-abi ABICoder dependency. (Update ABICoder dependency #3490)
Improve code clarity of HttpProvider keepAlive option setting. (Improve clarity of HttpProvider keepAlive value #3463)
Updated type definitions for Web3 HTTP Provider. (updated types for web3 http provider #3335 #3482)
Improve code clarity of HttpProvider keepAlive value. (Improve clarity of HttpProvider keepAlive value #3463)
Fix indentation of web3-eth documentation (Small fix on web3-eth.rst documentation #3478)

Fixed

Fix intermittent CI build issues with dtslint. (Fix CI build error from dtslint #3479)
Fix WSS credentials Base64 encoding (for browser) (Websocket Provider's btoa helper's context [browser only] #3508)
Fire provider "error" and/or "end" events when Websocket provider disconnects (Websocket Provider "error" and "end" events not fired when client disconnects #3485)
Remove web3-eth revert error return types to squash TS compilation errors (Fix TS promise error types in web3-eth #3496)
Backfill event subscriptions when WS connection dropped across multiple blocks (WebSocket reconnect - ws provider will miss some events. #3491)
Stop swallowing errors when WS connection has died after exhausting reconnection attempts (WebSocket reconnect doesn't throw error on maxAttempts #3492)

Type of change

Release

Checklist:

cgewecke · 2020-05-08T18:47:35Z

As part of the RC process, am doing some test installations in other projects and running their tests to see if everything.

installs properly
builds
tests pass

This list will be actively edited:

Project	Desc	Status
oz-test-helpers	Test utils. Subset of their suite is web3 contracts	✅
gnosis/dex-react	TS React app ~200 tests + webpack build + jest	✅
synthetix (buidler branch)	~1000 units (check ABICoder change)	✅

Implementation notes for future reference:

Synthetix yarn resolutions required for:
- nomiclabs/
- openzeppelin-solidity-2.3.0
- chainlink/
- truffle/
oz-test-helpers
- truffle/
- openzeppelin/
gnosis/dex-react
- walletconnect
- gnosis/pm

coveralls · 2020-05-08T18:56:32Z

Coverage increased (+0.05%) to 89.602% when pulling da8122c on release/1.2.8 into 4ec98d4 on 1.x.

cgewecke · 2020-05-13T18:47:47Z

Found a behavioral difference in the new abi decoder while running the synthetix unit tests,

Negative numbers passed as uint params are now flagged as "out of bounds". They used to be allowed.

The new behavior seems safer - perhaps we can just mention the difference in the release notes?

[EDIT] One case where this change might be problematic is for testing underflow logic itself, as here at openzeppelin/math. Wouldn't immediately impact them because they're on @truffle/contract and Web3@1.2.1 fwiw.

JS test

it('should revert when owner set the Target threshold to negative', async () => {
  const thresholdPercent = -1;
  await assert.revert(feePool.setTargetThreshold(thresholdPercent, { from: owner }));
});

Solidity
NB: the test above throws: Threshold too high, e.g. -1 is an underflow

function setTargetThreshold(uint _percent) external optionalProxy_onlyOwner {
        require(_percent >= 0, "Threshold should be positive"); 
        require(_percent <= 50, "Threshold too high");
        targetThreshold = _percent.mul(SafeDecimalMath.unit()).div(100);
}

Error

  1) Contract: FeePool
       when integrating with modern contracts
         FeeClaimablePenaltyThreshold
           should revert when owner set the Target threshold to negative:
     AssertionError: expected 'value out-of-bounds (argument="_percent", value=-1, code=INVALID_ARGUMENT, version=abi/5.0.0-beta.153)' to include 'revert'
      at Object.assertRevert [as revert] (test/utils/index.js:402:11)
      at process._tickCallback (internal/process/next_tick.js:68:7)

cgewecke · 2020-05-13T19:50:41Z

Looks like the some of the types changes might be breaking....

gnosis/dex-react tsc fails with:

ERROR in /Users/cgewecke/code/ef/synII/dex-react/src/api/deposit/DepositApi.ts
ERROR in /Users/cgewecke/code/ef/synII/dex-react/src/api/deposit/DepositApi.ts(70,31):
TS2352: Conversion of type 'Contract' to type 'ExtendedContract<BatchExchange>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'Contract' is not comparable to type 'Pick<BatchExchange, "methods" | "defaultAccount" | "defaultBlock" | "defaultCommon" | "defaultHardfork" | "defaultChain" | "transactionPollingTimeout" | "transactionConfirmationBlocks" | ... 4 more ... | "getPastEvents">'.
    Types of property 'methods' are incompatible.
      Type '{ [key: string]: ContractSendMethod; }' is missing the following properties from type '{ IMPROVEMENT_DENOMINATOR(): TransactionObject<string>; getSecondsRemainingInBatch(): TransactionObject<string>; requestWithdraw(token: string, amount: string | number): TransactionObject<...>; ... 37 more ...; getCurrentObjectiveValue(): TransactionObject<...>; }': IMPROVEMENT_DENOMINATOR, getSecondsRemainingInBatch, requestWithdraw, FEE_FOR_LISTING_TOKEN_IN_OWL, and 37 more.

ERROR in /Users/cgewecke/code/ef/synII/dex-react/src/api/erc20/Erc20Api.ts
ERROR in /Users/cgewecke/code/ef/synII/dex-react/src/api/erc20/Erc20Api.ts(106,31):
TS2352: Conversion of type 'Contract' to type 'ExtendedContract<Erc20>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'Contract' is not comparable to type 'Pick<Erc20, "methods" | "defaultAccount" | "defaultBlock" | "defaultCommon" | "defaultHardfork" | "defaultChain" | "transactionPollingTimeout" | "transactionConfirmationBlocks" | ... 4 more ... | "getPastEvents">'.
    Types of property 'methods' are incompatible.
      Type '{ [key: string]: ContractSendMethod; }' is missing the following properties from type '{ name(): TransactionObject<string>; approve(_spender: string, _value: string | number): TransactionObject<boolean>; totalSupply(): TransactionObject<...>; ... 5 more ...; allowance(_owner: string, _spender: string): TransactionObject<...>; }': name, approve, totalSupply, transferFrom, and 5 more.

ERROR in /Users/cgewecke/code/ef/synII/dex-react/src/api/tcr/MultiTcrApi.ts
ERROR in /Users/cgewecke/code/ef/synII/dex-react/src/api/tcr/MultiTcrApi.ts(37,31):
TS2352: Conversion of type 'Contract' to type 'ExtendedContract<Tcr>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'Contract' is not comparable to type 'Pick<Tcr, "methods" | "defaultAccount" | "defaultBlock" | "defaultCommon" | "defaultHardfork" | "defaultChain" | "transactionPollingTimeout" | "transactionConfirmationBlocks" | ... 4 more ... | "getPastEvents">'.
    Types of property 'methods' are incompatible.
      Property 'getTokens' is missing in type '{ [key: string]: ContractSendMethod; }' but required in type '{ getTokens(_listId: string | number): TransactionObject<string[]>; }'.

ERROR in /Users/cgewecke/code/ef/synII/dex-react/src/api/wallet/WalletApi.ts
ERROR in /Users/cgewecke/code/ef/synII/dex-react/src/api/wallet/WalletApi.ts(184,27):
TS2339: Property 'connected' does not exist on type 'HttpProvider | IpcProvider | WebsocketProvider | AbstractProvider'.
  Property 'connected' does not exist on type 'AbstractProvider'.
Child HtmlWebpackCompiler:
     1 asset
    Entrypoint HtmlWebpackPlugin_0 = __child-HtmlWebpackPlugin_0
    [0] ./node_modules/html-webpack-plugin/lib/loader.js!./src/html/index.html 1.74 KiB {0} [built]
Child favicons-webpack-plugin:
     1 asset
    Entrypoint logo.svg = 58d2a652c3e9d3d2d6b3
    [0] ./node_modules/cache-loader/dist/cjs.js?{"cacheDirectory":"/Users/cgewecke/code/ef/synII/dex-react/node_modules/.cache/favicons-webpack-plugin"}!./node_modules/favicons-webpack-plugin/src/loader.js?{"prefix":"assets/","options":{"appName":"Mesa - Gnosis Protocol DApp","appDescription":"Mesa - Gnosis Protocol DApp","developerName":"Mesa - Gnosis Protocol DApp","developerURL":null,"background":"#dfe6ef","themeColor":"#476481","icons":{"coast":false,"yandex":false},"version":"0.11.1"},"path":""}!./src/assets/img/logo.svg 3.34 MiB {0} [built]
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Can be tricked into passing by:

Adding an optional connected property to AbstractProvider here:

https://github.com/ethereum/web3.js/blob/d6eda4df04214fb2441b92fa56b856190e3629e5/packages/web3-core/types/index.d.ts#L419-L423

Appending | any to the contract.methods re-definition here:

https://github.com/ethereum/web3.js/blob/137ca7ba2167f5f022d18025ed3135c69476652e/packages/web3-eth-contract/types/index.d.ts#L49-L51

ricmoo · 2020-05-13T20:37:17Z

I think the version of the ABI coder Web3 was previously using was quite old, so there may be other important differences to try out.

bit widths; passing 257 in a uint8 should fail
bytesXX must be exactly the correct length; the older ethers coder may have automatically padded, depending on what version (e.g. bytes2 should be “0x1234”, not “0x12”)

alcuadrado · 2020-05-14T14:18:22Z

I think it's worth reconsidering the option of vendoring that part of ethers v4. Otherwise, the risk of accidentally introducing a breaking change is high.

cgewecke · 2020-05-14T15:48:50Z

@alcuadrado

Are there any precautions which could be taken which would make you feel more at ease with this?

The case for upgrading (e.g. not vendoring) is to make encoding/decoding here more closely aligned with ongoing work at Ethers and and inherit improvements there.

In two cases identified so far, web3 is accepting invalid inputs and a behavior change might be reasonable to consider as bug fixes:

throw when passing negative numbers as unsigned ints
throw when passing numbers outside allowable bit ranges for params

A third change is automatic padding for byte values - it looks like efforts here to address that might be incomplete (but could be fixed).

We've run the Synthetix and mosaicdao/mosaic unit tests (~1300) with 1.2.8-rc.0 and they're (basically) passing...

Definitely agree that there's risk and some unknowns here. But am wondering how you would upgrade this dependency at all given the versioning constraints here. Do you think on balance this is not worth doing?

alcuadrado · 2020-05-15T12:38:37Z

I'm ok with the upgrade @cgewecke. It will have to happen eventually.

I was just putting the other alternative on the table again, in case the upgrade path isn't complete fast enough. This was in the context of web3's future uncertainty and the npm stats issue. Otherwise, I wouldn't care about the time it takes to ship this change and just upgrade.

cgewecke · 2020-05-15T17:18:22Z

@ricmoo

From memory, does this seem like a complete list of the input validation changes between ABI Coder V4 / V5?

byte lengths enforced for bytesX and variants
bit widths enforced for uintX and variants
positive number enforced for uint

It looks like the approach we'd take here to minimize breaks is to pad bytes where possible and treat the additional strictness elsewhere as bug fixes.

Update AbstractProvider and contract.methods TS definitions

…r dep update

Update AbiCoder param formatting

ryanio · 2020-05-18T19:51:33Z

Thanks everyone. I have released 1.2.8-rc.1 with the following:

Update AbiCoder param formatting (#3522)
Update AbstractProvider and contract.methods TS definitions (#3521)
Fix setContenthash docs formatting (#3511)

cgewecke

Re-ran synthetix and gnosis/dex-react outside tests with 1.2.8-rc.1 and they're both good.

LGTM.

v1.2.8-rc.0

035d11a

ryanio added In Progress Currently being worked on Release labels May 8, 2020

ryanio requested review from alcuadrado, cgewecke, gnidan and michaelsbradleyjr May 8, 2020 18:59

Add optional connected property to AbstractProvider type

14a0272

cgewecke mentioned this pull request May 14, 2020

Update AbstractProvider and contract.methods TS definitions #3521

Merged

3 tasks

cgewecke added 2 commits May 14, 2020 13:24

Revert contract.method TS definition change (PR #3454)

7d27f50

Apply geth 1.9.13 CI fix to release branch

7911e5d

ryanio mentioned this pull request May 15, 2020

Update AbiCoder param formatting #3522

Merged

13 tasks

ryanio and others added 7 commits May 18, 2020 09:49

Merge pull request #3521 from ethereum/fix/release-tsc-errors

58d1f5b

Update AbstractProvider and contract.methods TS definitions

handle more encoding rules to reduce any breaking changes for abicode…

b7dbcd6

…r dep update

Add detail to compatibility comment

d13c468

Fix setContenthash docs formatting (#3511)

16c44d4

Merge pull request #3522 from ethereum/abiCoderParamFormatting

ba7677d

Update AbiCoder param formatting

Add 1.2.8-rc.1 changelog entries

fcf5d22

v1.2.8-rc.1

6289002

cgewecke approved these changes May 20, 2020

View reviewed changes

v1.2.8

e5b3b37

ryanio removed the In Progress Currently being worked on label May 20, 2020

Merge branch '1.x' into release/1.2.8

da8122c

ryanio merged commit 740a982 into 1.x May 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Release - 1.2.8 #3510

Release - 1.2.8 #3510

ryanio commented May 8, 2020 •

edited

Loading

cgewecke commented May 8, 2020 •

edited

Loading

coveralls commented May 8, 2020 •

edited

Loading

cgewecke commented May 13, 2020 •

edited

Loading

cgewecke commented May 13, 2020

ricmoo commented May 13, 2020

alcuadrado commented May 14, 2020 •

edited

Loading

cgewecke commented May 14, 2020 •

edited

Loading

alcuadrado commented May 15, 2020

cgewecke commented May 15, 2020

ryanio commented May 18, 2020

cgewecke left a comment

Release - 1.2.8 #3510

Release - 1.2.8 #3510

Conversation

ryanio commented May 8, 2020 • edited Loading

Description

Features

Added

Changed

Fixed

Type of change

Checklist:

cgewecke commented May 8, 2020 • edited Loading

coveralls commented May 8, 2020 • edited Loading

cgewecke commented May 13, 2020 • edited Loading

cgewecke commented May 13, 2020

ricmoo commented May 13, 2020

alcuadrado commented May 14, 2020 • edited Loading

cgewecke commented May 14, 2020 • edited Loading

alcuadrado commented May 15, 2020

cgewecke commented May 15, 2020

ryanio commented May 18, 2020

cgewecke left a comment

Choose a reason for hiding this comment

ryanio commented May 8, 2020 •

edited

Loading

cgewecke commented May 8, 2020 •

edited

Loading

coveralls commented May 8, 2020 •

edited

Loading

cgewecke commented May 13, 2020 •

edited

Loading

alcuadrado commented May 14, 2020 •

edited

Loading

cgewecke commented May 14, 2020 •

edited

Loading