Skip to content
Merged
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@lumerin/wallet-core",
"version": "1.0.73",
"version": "1.0.74",
"author": {
"name": "Lumerin",
"email": "developer@lumerin.io",
Expand Down
100 changes: 3 additions & 97 deletions src/plugins/eth/web3.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,13 @@
const logger = require('../../logger');

const Web3 = require('web3')
const https = require('https')
const { Web3Http } = require('./web3Http');

let providers = [];

function createWeb3(config) {
// debug.enabled = config.debug

providers = config.httpApiUrls.map((url) => {
return new Web3.providers.HttpProvider(url, {
agent: new https.Agent({
rejectUnauthorized: false, // Set to false if your HTTPS node endpoint uses a self-signed certificate
}),
})
})

const web3 = new Web3(providers[0], {
agent: new https.Agent({
rejectUnauthorized: false, // Set to false if your HTTPS node endpoint uses a self-signed certificate
}),
})

overrideFunctions(web3, providers)
overrideFunctions(web3.eth, providers)
web3.subscriptionProvider = subscriptionProvider

const web3 = new Web3Http(config.httpApiUrls)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The createWeb3 function creates a new instance of Web3Http using config.httpApiUrls. There is no validation of config.httpApiUrls before it's used. If it's not provided or is not in the expected format, this could lead to errors. Consider adding validation for config.httpApiUrls before using it.

return web3
}
Comment thread
alex-sandrk marked this conversation as resolved.
Comment on lines 9 to 14
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The createWeb3 function is creating a new instance of Web3Http and returning it, but there's no validation of the config parameter. If config.httpApiUrls is not provided or is not in the expected format, this could lead to unexpected behavior or errors. It's recommended to add validation for the config parameter and its properties before using them.

Comment on lines 9 to 14
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The createWeb3 function creates a new instance of Web3Http but does not use the Web3 module imported at line 5. If the Web3 module is not used elsewhere in the code, it should be removed to avoid confusion and unnecessary imports. If it is used, consider renaming the function or the variable to avoid confusion between the Web3 module and the web3 variable.


Expand Down Expand Up @@ -66,85 +48,9 @@ function createWeb3Subscribable(config, eventBus) {
}

function destroyWeb3(web3) {
web3.currentProvider.disconnect()
}

const urls = [
process.env.HTTP_ETH_NODE_ADDRESS,
process.env.HTTP_ETH_NODE_ADDRESS2,
process.env.HTTP_ETH_NODE_ADDRESS3,
]

let lastUsedProviderIndex = -1

const overrideFunctions = function (object, providers) {
const originalSetProvider = object.setProvider

const originalFunctions = Object.assign({}, object)
Object.keys(originalFunctions).forEach((key) => {
if (
typeof originalFunctions[key] === 'function' &&
!key.startsWith('set')
) {
object[key] = function () {
const originalFunction = originalFunctions[key]
const isAsync = originalFunction[Symbol.toStringTag] === 'AsyncFunction'
const args = arguments
let providerIndex = lastUsedProviderIndex
let result
do {
providerIndex = (providerIndex + 1) % providers.length
const provider = providers[providerIndex]
originalSetProvider(provider)
if (isAsync) {
result = originalFunction
.apply(this, args)
.then((res) => {
if (res !== undefined) {
return res
}
throw new Error('Result is undefined')
})
.catch((error) => {
console.error(`Error with provider ${provider.host}:`, error)
throw error
})
} else {
try {
if (new.target) {
function F(args) {
return originalFunction.apply(this, args)
}

F.prototype = originalFunction.prototype

return new F(args)
} else {
result = originalFunctions[key].apply(this, args)
}

if (typeof result !== "undefined") {
break
}
} catch (error) {
console.error(`Error with provider ${provider.host}:`, error)
}
}
} while (providerIndex !== lastUsedProviderIndex)
lastUsedProviderIndex = providerIndex
if (typeof result === "undefined") {
throw new Error('All providers failed to execute the function')
}
return result
}
}
})

object.setProvider = originalSetProvider
web3.currentProvider?.disconnect()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The destroyWeb3 function assumes that web3.currentProvider is always defined. If web3.currentProvider is undefined, calling disconnect() on it will throw an error. Consider adding error handling or a check to ensure that web3.currentProvider is defined before calling disconnect() on it.

}

let subscriptionProvider

module.exports = {
createWeb3,
destroyWeb3,
Comment thread
alex-sandrk marked this conversation as resolved.
Comment on lines 55 to 56
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The exported functions createWeb3 and destroyWeb3 are not documented. While this is not a critical issue, it's a good practice to document exported functions, especially in a module like this that could be used by other parts of the application. This can help other developers understand what these functions do, what parameters they expect, and what they return. Consider adding JSDoc comments or similar documentation for these functions.

Expand Down
68 changes: 68 additions & 0 deletions src/plugins/eth/web3Http.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
const Web3 = require('web3')
const https = require('https')
const logger = require('../../logger')

class Web3Http extends Web3 {
constructor(providers, options) {
super()

this.providers = providers.map(
(provider) =>
new Web3.providers.HttpProvider(provider, {
agent: new https.Agent({
rejectUnauthorized: false, // Set to false if your HTTPS node endpoint uses a self-signed certificate
Comment thread
alex-sandrk marked this conversation as resolved.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The rejectUnauthorized option is set to false which means that the application will not reject any unauthorized certificates. This can lead to potential security risks such as Man-in-the-Middle (MitM) attacks where an attacker can intercept and possibly alter the communication between the client and the server. It's recommended to set rejectUnauthorized to true in production environments to ensure that only authorized certificates are accepted. If you're using self-signed certificates for development, consider using a process environment variable to switch between settings.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The rejectUnauthorized option is set to false which means that the application will accept and establish a connection even if the server's SSL certificate is not trusted. This is a security risk as it makes the application vulnerable to man-in-the-middle attacks. The recommended solution is to use trusted certificates and set rejectUnauthorized to true. If self-signed certificates are necessary for development or internal use, consider using a configuration option to toggle this behavior, but ensure that in production environments, only trusted certificates are used.

}),
})
)
this.currentIndex = 0
this.retryCount = 0

// Initialize Web3 with the first provider from the list
this.setCustomProvider(this.providers[this.currentIndex])

// Set options if provided
if (options) {
this.setProviderOptions(options)
}
}

setCustomProvider(provider) {
// Override the setProvider method to handle switching providers on failure
this.setProvider(provider)

// Hook into provider's request and response handling
const originalSend = this.currentProvider.send.bind(this.currentProvider)
this.currentProvider.send = (payload, callback) => {
originalSend(payload, (error, response) => {
if (error) {
// Avoid infinite loop
if (this.retryCount >= this.providers.length) {
callback(error, null)
this.retryCount = 0
return;
}
// If the request fails, switch to the next provider and try again
this.currentIndex = (this.currentIndex + 1) % this.providers.length
this.setCustomProvider(this.providers[this.currentIndex])
logger.error(
`Switched to provider: ${this.providers[this.currentIndex].host}`
)
this.retryCount += 1
this.currentProvider.send(payload, callback) // Retry the request
} else {
this.retryCount = 0
callback(null, response)
}
})
}
Comment thread
alex-sandrk marked this conversation as resolved.
Comment on lines +35 to +57
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The retry mechanism implemented here could lead to a potential infinite loop if the error persists across all providers. Although there is a check to avoid an infinite loop if the retry count exceeds the number of providers, the retry count is reset to 0 before the callback is called with the error. If the callback function in turn calls the send method again, it could lead to an infinite loop. To avoid this, consider implementing a more robust retry mechanism that includes a maximum retry limit across all providers and does not reset the retry count before calling the error callback.

return true
}

setProviderOptions(options) {
this.currentProvider.host = options.host || this.currentProvider.host
this.currentProvider.timeout =
options.timeout || this.currentProvider.timeout
}
Comment thread
alex-sandrk marked this conversation as resolved.
Comment on lines +61 to +65
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The setProviderOptions method directly modifies the properties of this.currentProvider. This could potentially lead to unexpected behavior if other parts of the code rely on the original values of these properties. Instead of directly modifying these properties, consider creating a new provider with the updated options and setting it as the current provider. This way, you ensure that the original provider remains unchanged and can be reused if necessary.

Comment on lines +61 to +65
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The setProviderOptions method directly modifies the properties of this.currentProvider. This could potentially lead to unexpected behavior if the currentProvider object is used elsewhere in the application. Instead of directly modifying the properties of currentProvider, consider creating a new provider with the updated options and setting it as the current provider.

}

module.exports = { Web3Http }
139 changes: 139 additions & 0 deletions src/plugins/eth/web3Http.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
//@ts-check

const { expect } = require('chai')
const { Web3Http } = require('./web3Http')
const { Lumerin, CloneFactory } = require('contracts-js')

const invalidNode = 'https://arbitrum.llamarpc.com_INVALID'
const validNode = 'https://arbitrum.blockpi.network/v1/rpc/public'
const providerList = [
invalidNode,
validNode,
'https://rpc.ankr.com/arbitrum',
'https://arbitrum.api.onfinality.io/public',
'https://arb1-mainnet-public.unifra.io',
'https://arbitrum-one.public.blastapi.io',
'https://endpoints.omniatech.io/v1/arbitrum/one/public',
'https://1rpc.io/arb',
]

describe('Web3 multiple nodes integration tests', () => {
const lumerinAddress = '0x0FC0c323Cf76E188654D63D62e668caBeC7a525b'
const cloneFactoryAddress = '0x05C9F9E9041EeBCD060df2aee107C66516E2b9bA'

it('should work with simple blockchain query', async () => {
const web3 = new Web3Http(providerList)

const result = await web3.eth.getBlockNumber()
expect(typeof result).eq('number')
expect(web3.currentIndex).eq(1)
})

it('should iterate all nodes', async () => {
const web3 = new Web3Http([
invalidNode,
invalidNode,
invalidNode,
invalidNode,
validNode,
])

const result = await web3.eth.getBlockNumber()
expect(typeof result).eq('number')
expect(web3.currentIndex).eq(4)
})

it('should work with Contract.call()', async () => {
const web3 = new Web3Http(providerList)
const lumerin = Lumerin(web3, lumerinAddress)

const result = await lumerin.methods
.balanceOf('0x0000000000000000000000000000000000000000')
.call()
expect(typeof result).eq('string')
expect(web3.currentIndex).eq(1)
})

it('should work with Contract.send()', async () => {
const web3 = new Web3Http(providerList)
const cf = CloneFactory(web3, cloneFactoryAddress)

try {
await cf.methods
.setCreateNewRentalContract(
'0',
'0',
'0',
'0',
'0x0000000000000000000000000000000000000000',
'0'
)
.send({
from: '0x0000000000000000000000000000000000000000',
})
expect(1).eq(0)
} catch (err) {
expect(err.message.includes('unknown account')).eq(true)
}
})

it('should work with Contract.estimateGas()', async () => {
const web3 = new Web3Http(providerList)
const cf = CloneFactory(web3, cloneFactoryAddress)

try {
await cf.methods
.setCreateNewRentalContract(
'0',
'0',
'0',
'0',
'0x0000000000000000000000000000000000000000',
'0'
)
.estimateGas({
from: '0x0000000000000000000000000000000000000000',
})
expect(1).eq(0)
} catch (err) {
expect(err.message.includes('execution reverted')).eq(true)
expect(web3.currentIndex).eq(1)
}
})

it('should not iterate if request if invalid/reverted', async () => {
const web3 = new Web3Http([validNode, validNode, validNode])
const cf = CloneFactory(web3, cloneFactoryAddress)

try {
await cf.methods
.setCreateNewRentalContract(
'0',
'0',
'0',
'0',
'0x0000000000000000000000000000000000000000',
'0'
)
.send({
from: '0x0000000000000000000000000000000000000000',
})
expect(1).eq(0)
} catch (err) {
expect(err.message.includes('unknown account')).eq(true)
expect(web3.currentIndex).eq(0)
}
})

it('should not loop forever', async () => {
const web3 = new Web3Http([invalidNode, invalidNode, invalidNode])

try {
await web3.eth.getBlockNumber()
expect(1).eq(0)
} catch (err) {
expect(web3.retryCount).eq(0)
expect(web3.currentIndex).eq(0)
}
})
})