Remove web3 dependency, replace with ethers - #479
Open
marcos-iov wants to merge 8 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request migrates Rootstock/RSK bridge and related test utilities from Web3.js-style contract interactions to Ethers v6, including updates to transaction sending helpers, assertions, and raw JSON-RPC usage for RSK-specific methods.
Changes:
- Replaced
web3.eth.Contractusage withethers.Contractacross bridge-related helpers and tests, adopting direct calls and.staticCallpatterns. - Refactored tx-sending and assertion helpers to accept
(contract, methodName, args)rather than Web3methodobjects. - Introduced an Ethers-based helper for
rsk_getStorageBytesAtand removed the Web3 extension module.
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/01_powpeg/extra/99-register_flyover_btc_transaction.js | Updates flyover tests to Ethers contract properties/calls and new tx helper signature. |
| tests/01_powpeg/extra/99-register_fast_bridge_btc_below_minimum.js | Same migration as above for below-minimum scenario. |
| tests/01_powpeg/extra/99-flyover_sending_same_tx_without_witness_twice.js | Migrates repeated-send test to new call/tx patterns. |
| tests/01_powpeg/extra/99-flyover_sending_same_tx_with_witness_twice.js | Migrates segwit coinbase registration + repeated-send test to new helper signatures. |
| tests/01_powpeg/extra/13-registerFastBridgeBtcTransaction_user_call.js | Updates user-call test to Ethers; currently needs .staticCall for non-view call. |
| tests/01_powpeg/extra/12-block-header-precompile.js | Replaces Web3 contract and block queries with Ethers provider/contract calls and raw RPC for RSK fields. |
| tests/01_powpeg/extra/09-coinbase_information.js | Migrates coinbase registration test to new tx helper signature; needs correct witness merkle root argument. |
| tests/01_powpeg/extra/08-fed_pubkeys_fork.js | Updates assertion helpers to the new (contract, methodName, args) API. |
| tests/01_powpeg/extra/07-lock_whitelist_fork.js | Updates whitelist assertions to new contract-method assertion API. |
| tests/01_powpeg/extra/03-powpeg_redeem_script.js | Converts bridge view calls from Web3 .call() to Ethers direct view calls. |
| tests/01_powpeg/extra/02-fee_per_kb.js | Uses Ethers .staticCall for “dry-run” voting and direct calls for getters. |
| tests/01_powpeg/extra/01-vote_for_locking_cap_to_21m.js | Switches account import/unlock + bridge calls to the new helper APIs. |
| package.json | Adds ethers and updates bridge tx parser + rsk tx helper dependency versions. |
| lib/web3-utils.js | Removes the Web3-specific RSK RPC extension module. |
| lib/utils.js | Replaces Web3 keccak helper with Ethers keccak256. |
| lib/union-bridge-utils.js | Refactors event assertions to parse Ethers receipts/logs; needs safe parseLog handling. |
| lib/tests/union-bridge-methods.js | Migrates union-bridge tests to Ethers contracts, static calls, and new tx helper signatures. |
| lib/tests/get_estimated_fees_methods.js | Updates fee estimation tests to direct Ethers calls and new assertion API. |
| lib/tests/change-federation.js | Refactors federation-change flows to Ethers patterns and rsk_getStorageBytesAt helper. |
| lib/tests/call_receive_headers.js | Migrates receiveHeaders test to new sendTxWithCheck signature and direct getters. |
| lib/tests/call_receive_header.js | Migrates receiveHeader test and uses .staticCall for dry-run scenario. |
| lib/tests/bridge-calls.js | Migrates dynamic bridge method tests; needs .staticCall to preserve Web3 .call() semantics for non-view methods. |
| lib/tests/2wp.js | Updates several bridge calls and address checksum handling to Ethers patterns. |
| lib/sol-utils.js | Replaces Web3 deployment flow with ethers.ContractFactory + signer-based sending. |
| lib/rsk-utils.js | Refactors tx sending, mempool inspection, log decoding, and assertion helpers to Ethers; needs safe parseLog handling. |
| lib/rsk-tx-helper-provider.js | Removes Web3 extension setup and instantiates RskTransactionHelper v6 directly. |
| lib/rsk-rpc-utils.js | Adds raw JSON-RPC helper for rsk_getStorageBytesAt; should normalize numeric block tags. |
| lib/liquidity-bridge-contract.js | Updates liquidity bridge deployment/contract access to Ethers (provider swap via new Contract). |
| lib/federation-utils.js | Migrates federation info/public key getters to Ethers direct calls. |
| lib/bridge-provider.js | Replaces bridge instantiation with new ethers.Contract(precompileAddress, abi, provider). |
| lib/assertions/whitelisting.js | Migrates whitelist assertions to new tx helper signatures and Ethers .staticCall. |
| lib/assertions/contractMethods.js | Refactors contract call assertion helpers to Ethers .staticCall. |
| lib/2wp-utils.js | Migrates bridge-related helpers to Ethers and introduces getStorageBytesAt usage; updates sendTxToBridge return contract. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+950
to
954
| const parsedLog = contractInterface.parseLog(log); | ||
| if (!parsedLog) { | ||
| continue; | ||
| } | ||
| const event = decodeLog(rskClient, log, abiElement); | ||
| events.push(event); | ||
| } | ||
| return events; | ||
| }; | ||
|
|
||
| const buildEventSignatureMap = (rskClient, contractAbi) => { | ||
| return contractAbi | ||
| .flat() | ||
| .filter((element) => element.type === 'event') | ||
| .reduce((acc, element) => { | ||
| const signature = rskClient.eth.abi.encodeEventSignature(element); | ||
| acc[signature] = element; | ||
| return acc; | ||
| }, {}); | ||
| }; | ||
|
|
Comment on lines
+21
to
+29
| const findEventInReceipt = (contract, txReceipt, eventName) => { | ||
| for (const log of txReceipt.logs) { | ||
| const parsedLog = contract.interface.parseLog(log); | ||
| if (parsedLog && parsedLog.name === eventName) { | ||
| return parsedLog; | ||
| } | ||
| } | ||
| return undefined; | ||
| }; |
Comment on lines
66
to
+70
| ensure0x(blockHash[0]), | ||
| ensure0x(pmt), | ||
| ensure0x(witnessReservedValue), | ||
| ensure0x(witnessReservedValue) | ||
| ); | ||
|
|
||
| const txReceipt = await rskUtils.sendTransaction( | ||
| rskTxHelper, | ||
| registerBtcCoinbaseTransactionMethod, | ||
| ensure0x(witnessReservedValue), | ||
| ], |
Comment on lines
+1
to
+12
| /** | ||
| * Calls the RSK-specific `rsk_getStorageBytesAt` JSON-RPC method, which reads raw storage bytes | ||
| * (as opposed to the single 32-byte word the standard `eth_getStorageAt` returns). | ||
| * @param {import('ethers').JsonRpcProvider} client | ||
| * @param {string} address the contract address to read storage from | ||
| * @param {string} storageIndex the storage index/key, as a 0x-prefixed hex string | ||
| * @param {string|number} blockNumber defaults to 'latest' | ||
| * @returns {Promise<string>} the storage value, RLP-encoded as a 0x-prefixed hex string | ||
| */ | ||
| const getStorageBytesAt = (client, address, storageIndex, blockNumber = 'latest') => { | ||
| return client.send('rsk_getStorageBytesAt', [address, storageIndex, blockNumber]); | ||
| }; |
Comment on lines
93
to
97
| it('normal call works', async () => { | ||
| try { | ||
| const result = await bridgeMethod.call(); | ||
| const result = await bridge[methodName](...args); | ||
|
|
||
| if (expectedWhenFail === null) { |
Comment on lines
77
to
+81
| * @param {boolean} mine If true, mines 1 block after sending the transaction. If false, it will not mine the tx and will return undefined. Defaults to true. | ||
| * @returns {Promise<web3.eth.TransactionReceipt | txPromise>} the rsk tx receipt if `mine` is true, otherwise the tx promise. | ||
| * @returns {Promise<import('ethers').TransactionReceipt | string>} the rsk tx receipt if `mine` is true, otherwise the tx hash. | ||
| */ | ||
| const sendTxToBridge = async (rskTxHelper, amountInWeisBN, rskFromAddress, mine = true) => { | ||
| const txPromise = rskTxHelper.sendTransaction({ | ||
| const txHash = await rskTxHelper.sendTransaction({ |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
lib/union-bridge-utils.js:25
contract.interface.parseLog(log)throws for logs that don't match the contract ABI (and receipts can include logs from multiple contracts due to internal calls). As written, this will throw when it encounters a non-authorizer log (e.g., Bridge logs in the same receipt), so event assertions will fail. Filter bylog.addressand/or wrapparseLogin a try/catch and continue on non-matching logs.
const findEventInReceipt = (contract, txReceipt, eventName) => {
for (const log of txReceipt.logs) {
const parsedLog = contract.interface.parseLog(log);
if (parsedLog && parsedLog.name === eventName) {
return parsedLog;
tests/01_powpeg/extra/13-registerFastBridgeBtcTransaction_user_call.js:33
registerFastBridgeBtcTransactionis not a view function, so callingbridge.registerFastBridgeBtcTransaction(...)will try to send a transaction (and will fail without a signer). The previousweb3code used.call()semantics; in ethers this should be.staticCall(...)to simulate the call and get the response code.
const callResult = await bridge.registerFastBridgeBtcTransaction(
'0x',
1,
stringHex,
stringHex,
lib/rsk-utils.js:952
Interface.parseLog()throws when the log doesn't match the provided ABI (it does not returnnull). With the current loop, any unrelated log (or an emptycontractAbi, as used byfindEventInTxdefaults) can makedecodeLogsthrow and break callers. WrapparseLogin a try/catch and continue on non-matching logs.
}
const parsedLog = contractInterface.parseLog(log);
if (!parsedLog) {
continue;
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
lib/union-bridge-utils.js:24
contract.interface.parseLog(log)throws when the log doesn't match any event in the interface. As written, the first unrelated log in the receipt will throw and abort the search, so event assertions become flaky whenever the tx emits logs from multiple contracts. WrapparseLogin a try/catch and continue on non-matching logs.
const findEventInReceipt = (contract, txReceipt, eventName) => {
for (const log of txReceipt.logs) {
const parsedLog = contract.interface.parseLog(log);
if (parsedLog && parsedLog.name === eventName) {
lib/rsk-utils.js:953
Interface.parseLogthrows for logs that don't match any event in the ABI. Since receipts often include logs from multiple contracts, this will throw and prevent any event decoding. Catch parse failures and skip non-matching logs.
const parsedLog = contractInterface.parseLog(log);
if (!parsedLog) {
continue;
}
tests/01_powpeg/extra/13-registerFastBridgeBtcTransaction_user_call.js:38
registerFastBridgeBtcTransactionis a state-changing bridge method; calling it asbridge.registerFastBridgeBtcTransaction(...)will try to send a transaction and requires a signer (the bridge instance here is connected only to a provider). The previous web3 version used.call()to simulate and read the return code, so this should be a static call in ethers.
const callResult = await bridge.registerFastBridgeBtcTransaction(
'0x',
1,
stringHex,
stringHex,
btcAddressBytes,
randomAddress,
btcAddressBytes,
false
);
Comment on lines
109
to
111
| it(`contract calls allowed`, async () => { | ||
| const success = await contractCallsTester.methods.doCall(abi).call(); | ||
| const success = await contractCallsTester.doCall(abi); | ||
| expect(success).to.be.true; |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
lib/union-bridge-utils.js:25
contract.interface.parseLog(log)throws for logs that don’t match the contract’s ABI (which is common in a receipt containing logs from multiple contracts). As written, this will throw on the first non-matching log and prevent finding the target event. WrapparseLogin a try/catch and continue on non-matching logs.
const findEventInReceipt = (contract, txReceipt, eventName) => {
for (const log of txReceipt.logs) {
const parsedLog = contract.interface.parseLog(log);
if (parsedLog && parsedLog.name === eventName) {
return parsedLog;
lib/rsk-utils.js:961
Interface.parseLogthrows when the log’s topic0 doesn’t match any event incontractAbi. Since receipts often include logs from other contracts, this will throw and stop decoding entirely. WrapparseLogin a try/catch and skip non-matching logs.
const parsedLog = contractInterface.parseLog(log);
if (!parsedLog) {
continue;
}
| */ | ||
| const callOrNullIfEmpty = async (contract, methodName, ...args) => { | ||
| try { | ||
| return await contract[methodName](...args); |
| */ | ||
| const callOrNullIfEmpty = async (contract, methodName, ...args) => { | ||
| try { | ||
| return await contract[methodName](...args); |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
lib/union-bridge-utils.js:25
contract.interface.parseLog(log)throws for logs that don’t match the contract ABI (e.g., logs from other contracts in the same receipt). As written,findEventInReceiptwill throw instead of skipping non-matching logs, causing false failures when receipts contain unrelated logs.
const findEventInReceipt = (contract, txReceipt, eventName) => {
for (const log of txReceipt.logs) {
const parsedLog = contract.interface.parseLog(log);
if (parsedLog && parsedLog.name === eventName) {
return parsedLog;
lib/rsk-utils.js:968
ethers.Interface.parseLogthrows when the log doesn’t match the provided ABI (it does not returnnull). Without a try/catch,decodeLogswill throw as soon as it encounters any unrelated log in the receipt, breaking event scanning.
const parsedLog = contractInterface.parseLog(log);
if (!parsedLog) {
continue;
}
tests/01_powpeg/extra/09-coinbase_information.js:70
registerBtcCoinbaseTransactionis being called withwitnessReservedValuefor both the 4th and 5th arguments. In other tests in this PR (e.g.99-flyover_sending_same_tx_with_witness_twice.js) the 4th argument iswitnessMerkleRoot, so this call is likely supplying the wrong parameter and may register incorrect coinbase info.
[
ensure0x(coinbaseTxWithoutWitness.toHex()),
ensure0x(blockHash[0]),
ensure0x(pmt),
ensure0x(witnessReservedValue),
ensure0x(witnessReservedValue),
],
Comment on lines
259
to
267
| const getRskMempoolTransactionsToTheBridge = async (rskTxHelper, withTxsDecoded = false) => { | ||
| const mempoolBlock = await rskTxHelper.getClient().eth.getBlock('pending', withTxsDecoded); | ||
| const transactionsToTheBridge = mempoolBlock.transactions.filter( | ||
| (tx) => tx.to === BRIDGE_ADDRESS | ||
| ); | ||
| const mempoolBlock = await rskTxHelper.getClient().getBlock('pending', withTxsDecoded); | ||
| // `Block.transactions` is always an array of tx hashes; the full tx objects (`.to`, `.data`, etc.) | ||
| // requested via `withTxsDecoded` are only available through `prefetchedTransactions`. | ||
| const transactions = withTxsDecoded | ||
| ? mempoolBlock.prefetchedTransactions | ||
| : mempoolBlock.transactions; | ||
| const transactionsToTheBridge = transactions.filter((tx) => tx.to === BRIDGE_ADDRESS); | ||
| return transactionsToTheBridge; |
Comment on lines
+82
to
+96
| /** | ||
| * Calls `contract[methodName](...args)` as a static (read) call, treating an ethers `BAD_DATA` | ||
| * decode failure (the node returning empty `0x` for an unset `bytes` value) as `null`, matching | ||
| * the previous web3 behavior for these same calls. | ||
| */ | ||
| const callOrNullIfEmpty = async (contract, methodName, ...args) => { | ||
| try { | ||
| return await contract[methodName](...args); | ||
| } catch (error) { | ||
| if (error.code === 'BAD_DATA' && error.value === '0x') { | ||
| return null; | ||
| } | ||
| throw error; | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
lib/union-bridge-utils.js:28
contract.interface.parseLog(log)throws when the log doesn't match any event in the contract ABI. Since receipts typically include logs from multiple contracts, this helper can throw on the first unrelated log and prevent scanning the rest. Wrap the parse in a try/catch and skip logs that don't decode.
const findEventInReceipt = (contract, txReceipt, eventName) => {
for (const log of txReceipt.logs) {
const parsedLog = contract.interface.parseLog(log);
if (parsedLog?.name === eventName) {
return parsedLog;
}
}
return undefined;
lib/rsk-utils.js:964
Interface.parseLogthrows for logs whose first topic isn't an event in the provided ABI. As written,decodeLogswill throw when the receipt contains logs from other contracts (common for bridge calls). Catch decode errors and skip non-matching logs so callers likefindEventInTxdon't fail unexpectedly.
const parsedLog = contractInterface.parseLog(log);
if (!parsedLog) {
continue;
}
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
lib/rsk-utils.js:969
Interface.parseLogthrows when a log doesn’t match any event in the provided ABI (it does not returnnull/undefined). As written, a single unrelated log in the receipt will crashdecodeLogs()/findEventInTx()instead of being ignored (previous web3 implementation skipped unknown signatures). WrapparseLogin a try/catch and continue on failures.
const parsedLog = contractInterface.parseLog(log);
lib/union-bridge-utils.js:24
contract.interface.parseLog(log)throws for logs that don’t match the contract ABI (e.g., logs from other contracts in the same receipt). Without guarding, this helper can throw before it ever reaches the target event. Filter bylog.addressand/or wrapparseLogin try/catch so non-matching logs are skipped.
const findEventInReceipt = (contract, txReceipt, eventName) => {
for (const log of txReceipt.logs) {
const parsedLog = contract.interface.parseLog(log);
if (parsedLog?.name === eventName) {
lib/tests/bridge-calls.js:111
doCallincontracts/contract-calls-tester.solis not markedconstant/view, so in ethers it is treated as a state-changing method andcontractCallsTester.doCall(abi)will try to send a transaction (and will fail here becausecompileAndDeploy()returns a contract connected only to a provider, not a signer). This test previously usedweb3 ... .call()semantics; usestaticCallto keep it aneth_call.
it(`contract calls allowed`, async () => {
const success = await contractCallsTester.doCall(abi);
expect(success).to.be.true;
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
lib/rsk-utils.js:972
ethers.Interface.parseLogthrows when the log doesn’t match any event in the provided ABI (it doesn’t returnnull). As written,decodeLogswill crash on the first non-matching log in the receipt, breaking callers likefindEventInTxwhen the tx emits unrelated logs. WrapparseLogin a try/catch and skip unknown logs.
const parsedLog = contractInterface.parseLog(log);
if (!parsedLog) {
continue;
}
lib/union-bridge-utils.js:26
contract.interface.parseLog(log)throws for logs that aren’t part of the contract ABI. Since receipts typically include logs from multiple contracts, this helper can throw before it ever reaches the target event. Catch parse failures and continue scanning.
for (const log of txReceipt.logs) {
const parsedLog = contract.interface.parseLog(log);
if (parsedLog?.name === eventName) {
return parsedLog;
}
tests/01_powpeg/extra/09-coinbase_information.js:70
registerBtcCoinbaseTransactionexpects thewitnessMerkleRootandwitnessReservedValueas separate arguments (see other tests). This test currently passeswitnessReservedValuefor both, which makes the call invalid for witness-enabled blocks. For a freshly mined empty block (coinbase-only), the witness merkle root is 32 bytes of zero.
[
ensure0x(coinbaseTxWithoutWitness.toHex()),
ensure0x(blockHash[0]),
ensure0x(pmt),
ensure0x(witnessReservedValue),
ensure0x(witnessReservedValue),
],
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors the codebase to migrate from using Web3.js contract calls to Ethers.js for interacting with the RSK Bridge contract. The changes modernize contract interaction patterns, improve type safety, and standardize method invocation throughout the codebase. Additionally, utility and assertion functions are updated to align with the new contract interaction style.
Migration to Ethers.js for Bridge Contract Interaction:
getBridge, replacing the previous Web3.js implementation. All contract method calls now use direct method calls or.staticCallfor view functions, instead of the.methods.<method>().call()pattern. (lib/bridge-provider.js,lib/2wp-utils.js,lib/federation-utils.js,lib/assertions/whitelisting.js,lib/assertions/contractMethods.js) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19]Refactoring of Utility and Assertion Functions:
lib/2wp-utils.js,lib/federation-utils.js) [1] [2] [3] [4] [5].staticCallfor read-only operations and to improve test clarity and maintainability. (lib/assertions/contractMethods.js,lib/assertions/whitelisting.js) [1] [2] [3] [4] [5] [6] [7] [8]API and Return Value Adjustments:
lib/2wp-utils.js)Code Simplification and Cleanup:
lib/2wp-utils.js,lib/federation-utils.js,lib/assertions/whitelisting.js) [1] [2] [3] [4] [5] [6]These changes collectively modernize the codebase, improve maintainability, and lay the groundwork for future enhancements using Ethers.js.