Skip to content

Commit a3972cf

Browse files
blueoginsirpy
andauthored
Fix: fix topAdmins function to use KMS wallet signing (#518)
* refactor: streamline topAdmins transaction handling in Web3Wallet and enhance logging for KMS configuration * chore: remove unnecessary console log for chainId in Web3Wallet to clean up code * refactor: enhance sendTransaction method in Web3Wallet to support optional 'from' address parameter and improve transaction locking logic --------- Co-authored-by: sirpy <hadar@gooddollar.org>
1 parent 780c9dd commit a3972cf

2 files changed

Lines changed: 21 additions & 31 deletions

File tree

src/server/blockchain/Web3Wallet.js

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -506,31 +506,17 @@ export class Web3Wallet {
506506
const { log } = this
507507

508508
try {
509-
const { nonce, release, fail, address } = await this.txManager.lock(this.addresses[0], 500) // timeout of 1 sec, so all "workers" fail except for the first
510-
511-
try {
512-
log.debug('topAdmins:', { numAdmins, address, nonce })
513-
for (let i = 0; i < numAdmins; i += 50) {
514-
log.debug('topAdmins sending tx', { address, nonce, adminIdx: i })
515-
const tx = this.proxyContract.methods.topAdmins(i, i + 50)
516-
const gas = await tx
517-
.estimateGas()
518-
.then(gas => parseInt(gas) + 200000) //buffer for proxy contract, reimburseGas?
519-
.catch(() => 1000000)
520-
await this.proxyContract.methods.topAdmins(i, i + 50).send({
521-
gas,
522-
maxFeePerGas: this.maxFeePerGas,
523-
maxPriorityFeePerGas: this.maxPriorityFeePerGas,
524-
from: address,
525-
nonce
526-
})
527-
log.debug('topAdmins success', { adminIdx: i })
528-
}
529-
530-
release()
531-
} catch (e) {
532-
log.error('topAdmins failed', e)
533-
fail()
509+
log.debug('topAdmins:', { numAdmins, addresses: this.addresses })
510+
for (let i = 0; i < numAdmins; i += 50) {
511+
log.debug('topAdmins sending tx', { adminIdx: i })
512+
await this.sendTransaction(
513+
this.proxyContract.methods.topAdmins(i, i + 50),
514+
{},
515+
{ from: this.addresses[0] },
516+
true,
517+
log
518+
)
519+
log.debug('topAdmins success', { adminIdx: i })
534520
}
535521
} catch (e) {
536522
log.error('topAdmins failed', e)
@@ -1302,7 +1288,8 @@ export class Web3Wallet {
13021288
5, // Goerli
13031289
80001, // Mumbai (Polygon testnet)
13041290
122, // Fuse
1305-
42220 // Celo
1291+
42220, // Celo
1292+
4447 // Local Testnet
13061293
// Note: XDC (50) will also become EIP-1559 soon and should be added when it's live
13071294
])
13081295

@@ -1558,15 +1545,17 @@ export class Web3Wallet {
15581545
* @param {number} gasValues.gas
15591546
* @param {number} gasValues.maxFeePerGas
15601547
* @param {number} gasValues.maxPriorityFeePerGas
1548+
* @param {string} [gasValues.from] - If set, lock and send from this address only; otherwise use this.filledAddresses
15611549
* @returns {Promise<Promise|Q.Promise<any>|Promise<*>|Promise<*>|Promise<*>|*>}
15621550
*/
15631551
async sendTransaction(
15641552
tx: any,
15651553
txCallbacks: PromiEvents = {},
1566-
{ gas, maxPriorityFeePerGas, maxFeePerGas, gasPrice }: GasValues = {
1554+
{ gas, maxPriorityFeePerGas, maxFeePerGas, gasPrice, from }: GasValues = {
15671555
gas: undefined,
15681556
maxFeePerGas: undefined,
1569-
maxPriorityFeePerGas: undefined
1557+
maxPriorityFeePerGas: undefined,
1558+
from: undefined
15701559
},
15711560
retry = true,
15721561
customLogger = null
@@ -1598,9 +1587,10 @@ export class Web3Wallet {
15981587
maxFeePerGas = normalizedGas.maxFeePerGas
15991588
maxPriorityFeePerGas = normalizedGas.maxPriorityFeePerGas
16001589

1601-
logger.trace('getting tx lock:', { txuuid })
1590+
logger.trace('getting tx lock:', { txuuid, fromOption: from })
16021591

1603-
const { nonce, release, address } = await this.txManager.lock(this.filledAddresses)
1592+
const addressesToLock = from != null ? [from] : this.filledAddresses
1593+
const { nonce, release, address } = await this.txManager.lock(addressesToLock)
16041594

16051595
logger.trace('got tx lock:', { txuuid, address })
16061596

src/server/blockchain/__tests__/adminWalletKMS.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ describe('AdminWallet KMS Transaction Submission', () => {
4545
describe('KMS Configuration', () => {
4646
test('should detect if KMS is configured', () => {
4747
const configured = isKMSConfigured()
48-
console.log('KMS is configured with key IDs:', conf.kmsKeysTag)
48+
console.log('KMS is configured with key tag:', conf.kmsKeysTag)
4949
expect(configured).toBe(true)
5050
})
5151

0 commit comments

Comments
 (0)