Skip to content

fix: HRC20 burn/burnFrom BNish type and BridgeToken devnet multisig signer - #78

Open
D2758695161 wants to merge 1 commit into
blockcoders:mainfrom
D2758695161:fix-hrc20-burn-types
Open

fix: HRC20 burn/burnFrom BNish type and BridgeToken devnet multisig signer#78
D2758695161 wants to merge 1 commit into
blockcoders:mainfrom
D2758695161:fix-hrc20-burn-types

Conversation

@D2758695161

Copy link
Copy Markdown

Description

Fixed two bugs in the Harmony Marketplace SDK:

Bug 1: HRC20 burn/burnFrom parameter type mismatch

The burn() and burnFrom() methods in HRC20 class had their amount parameter typed as number, but the HRC20 standard and the SDK README documentation specify that amounts should accept BNish type (BN | Arrayish | bigint | number).

Before:

public burn(amount: number, txOptions?: ITransactionOptions) {
  return this.send("burn", [amount], txOptions)
}
public burnFrom(account: string, amount: number, txOptions?: ITransactionOptions) {
  return this.send("burnFrom", [account, amount], txOptions)
}

After:

public burn(amount: BNish, txOptions?: ITransactionOptions): Promise<Transaction> {
  return this.send("burn", [amount], txOptions)
}
public burnFrom(account: string, amount: BNish, txOptions?: ITransactionOptions): Promise<Transaction> {
  return this.send("burnFrom", [account, amount], txOptions)
}

Also added missing Promise return type annotations.

Bug 2: BridgeToken devnet multisig signer always uses mainnet address

In the BridgeToken constructor, hmyMasterWallet.setSigner() was always called with MAINNET_MULTISIG_WALLET regardless of whether the network was mainnet or devnet.

Before:

this.hmyMasterWallet.setSigner(MAINNET_MULTISIG_WALLET)

After:

this.hmyMasterWallet = this.hmyMasterWallet.setSigner(
  this.isMainnet ? MAINNET_MULTISIG_WALLET : DEVNET_MULTISIG_WALLET,
)

Bounty claim: This PR addresses the coding errors bug bounty for Harmony Marketplace SDK (harmony-one/bounties#125).

Payment address for bounty: 0xaae0101ac77a2e4e0ea826eb4d309374f029b0a6

…tisig signer

Bug fixes:
1. HRC20.burn() and HRC20.burnFrom() parameters were typed as 'number'
   but should accept BNish (BN|string|bigint|Arrayish) per the README docs.
   Added Promise<Transaction> return types.

2. BridgeToken constructor always set hmyMasterWallet signer to MAINNET_MULTISIG_WALLET
   regardless of network. Fixed to use DEVNET_MULTISIG_WALLET for devnet.
@D2758695161

Copy link
Copy Markdown
Author

👋 Just checking in on this PR. Happy to make any adjustments if needed. Ready for merge whenever you are!

@D2758695161

Copy link
Copy Markdown
Author

👋 Hi! Just bumping this PR. All ready for merge - let me know if you need any changes! 🦞

@D2758695161

Copy link
Copy Markdown
Author

Hi @owner, my PR #78 (HRC20 fix) is still open and ready for review. Could you please take a look? Bounty payment address: 0xaae0101ac77a2e4e0ea826eb4d309374f029b0a6 (ERC20/BSC/ETH). Thank you!

@D2758695161

Copy link
Copy Markdown
Author

Gentle bump - PR ready to merge. Bounty: TBD USDT ? �xaae0101ac77a2e4e0ea826eb4d309374f029b0a6. Please review.

@D2758695161

Copy link
Copy Markdown
Author

?? Bump - this PR is ready to merge. Please let me know if you need any changes or have questions. Happy to iterate!

@D2758695161

Copy link
Copy Markdown
Author

@blockcoders — PR #78 fixes HRC20 burn/burnFrom BNish type and BridgeToken devnet signer. Ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant