Skip to content

Commit 35a62cc

Browse files
TomasvrbaTomasvrba
committed
eth: parse ERC-681 payment requests
Correctly parse native ETH and ERC20 payment requests scanned from QR codes. Use the request recipient and replace the entered amount only when the request includes one. Expose the chain ID and token decimals needed to verify and format requests. Reject requests for a different chain or token contract, and validate ERC20 amount bounds in the backend to avoid invalid transaction data.
1 parent 6f735e5 commit 35a62cc

10 files changed

Lines changed: 399 additions & 30 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
- Add option to navigate to "Used addresses" in sign-message workflow
1414
- Floating mobile bottom navigation bar
1515
- Mobile: Move settings into bottom navigation
16+
- Ethereum: improve QR code scanning and fix ERC20 QR payment requests
1617

1718
## v4.51.4
1819
- Bundle BitBox02 and BitBox02 Nova firmware version v9.26.5

‎backend/coins/eth/account.go‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,14 @@ func (account *Account) newTx(args *accounts.TxProposalArgs) (*TxProposal, error
546546
}
547547
value = parsedAmount.BigInt()
548548
}
549+
if account.coin.erc20Token != nil {
550+
if value.BitLen() > 256 {
551+
return nil, errp.WithStack(errors.ErrInvalidAmount)
552+
}
553+
if value.Cmp(account.balance.BigInt()) == 1 {
554+
return nil, errp.WithStack(errors.ErrInsufficientFunds)
555+
}
556+
}
549557

550558
var message ethereum.CallMsg
551559

@@ -579,16 +587,6 @@ func (account *Account) newTx(args *accounts.TxProposalArgs) (*TxProposal, error
579587
}
580588
}
581589

582-
// For ERC20 transfers, the EstimateGas call fails if we try to spend more than we have and we
583-
// do not have enough ether to pay the fee.
584-
// We make some checks upfront to catch this before calling out to the node and failing.
585-
if !args.Amount.SendAll() {
586-
if account.coin.erc20Token != nil {
587-
if value.Cmp(account.balance.BigInt()) == 1 {
588-
return nil, errp.WithStack(errors.ErrInsufficientFunds)
589-
}
590-
}
591-
}
592590
gasLimit, err := account.coin.client.EstimateGas(context.TODO(), message)
593591
if err != nil {
594592
if strings.Contains(err.Error(), etherscan.ERC20GasErr) {
@@ -601,14 +599,7 @@ func (account *Account) newTx(args *accounts.TxProposalArgs) (*TxProposal, error
601599
fee := new(big.Int).Mul(new(big.Int).SetUint64(gasLimit), suggestedGasFeeCap)
602600

603601
// Adjust amount with fee
604-
if account.coin.erc20Token != nil {
605-
// in erc 20 tokens, the amount is in the token unit, while the fee is in ETH, so there is
606-
// no issue withSendAll.
607-
608-
if !args.Amount.SendAll() && value.Cmp(account.balance.BigInt()) == 1 {
609-
return nil, errp.WithStack(errors.ErrInsufficientFunds)
610-
}
611-
} else {
602+
if account.coin.erc20Token == nil {
612603
if args.Amount.SendAll() {
613604
// Set the value correctly and check that the fee is smaller than or equal to the balance.
614605
value = new(big.Int).Sub(account.balance.BigInt(), fee)

‎backend/coins/eth/account_test.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/BitBoxSwiss/bitbox-wallet-app/backend/accounts"
1414
"github.com/BitBoxSwiss/bitbox-wallet-app/backend/accounts/errors"
1515
"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/coin"
16+
"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/eth/erc20"
1617
"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/eth/rpcclient"
1718
"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/eth/rpcclient/mocks"
1819
ethtypes "github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/eth/types"
@@ -196,6 +197,23 @@ func TestTxProposal(t *testing.T) {
196197
})
197198
}
198199

200+
func TestERC20TxProposalRejectsAmountOverflow(t *testing.T) {
201+
acct := newAccount(t)
202+
defer acct.Close()
203+
acct.coin.erc20Token = erc20.NewToken("0x89205a3a3b2a69de6dbf7f01ed13b2108b2c43e7", 0)
204+
require.NoError(t, acct.Update(big.NewInt(1e18), big.NewInt(100), nil))
205+
require.Eventually(t, acct.Synced, time.Second, time.Millisecond*200)
206+
207+
_, _, _, err := acct.TxProposal(&accounts.TxProposalArgs{
208+
RecipientAddress: "0xa29163852021BF4C139D03Dff59ae763AC73e84e",
209+
Amount: coin.NewSendAmount(
210+
"115792089237316195423570985008687907853269984665640564039457584007913129639936"),
211+
FeeTargetCode: accounts.FeeTargetCodeCustom,
212+
CustomFee: "20",
213+
})
214+
require.Equal(t, errors.ErrInvalidAmount, errp.Cause(err))
215+
}
216+
199217
func newTestOutgoingTx() *gethtypes.Transaction {
200218
to := common.HexToAddress("0xa29163852021BF4C139D03Dff59ae763AC73e84e")
201219
return gethtypes.NewTx(&gethtypes.LegacyTx{

‎backend/handlers/account_json_test.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
package handlers
4+
5+
import (
6+
"strings"
7+
"testing"
8+
9+
coinpkg "github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/coin"
10+
"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/eth"
11+
"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/eth/erc20"
12+
"github.com/BitBoxSwiss/bitbox-wallet-app/backend/config"
13+
"github.com/ethereum/go-ethereum/params"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestEthereumAccountJSONIncludesPaymentRequestMetadata(t *testing.T) {
18+
accountCoin := eth.NewCoin(
19+
nil,
20+
coinpkg.CodeETH,
21+
"Ethereum",
22+
"ETH",
23+
"ETH",
24+
params.MainnetChainConfig,
25+
"",
26+
nil,
27+
nil,
28+
)
29+
30+
account := newAccountJSON(config.Keystore{}, &config.Account{}, accountCoin, nil, false)
31+
32+
require.Equal(t, "1", account.ChainID)
33+
require.NotNil(t, account.Decimals)
34+
require.Equal(t, uint(18), *account.Decimals)
35+
}
36+
37+
func TestERC20AccountJSONIncludesPaymentRequestMetadata(t *testing.T) {
38+
accountCoin := eth.NewCoin(
39+
nil,
40+
"erc20Test",
41+
"Test token",
42+
"TOK",
43+
"ETH",
44+
params.MainnetChainConfig,
45+
"",
46+
nil,
47+
erc20.NewToken("0x89205a3a3b2a69de6dbf7f01ed13b2108b2c43e7", 6),
48+
)
49+
50+
account := newAccountJSON(config.Keystore{}, &config.Account{}, accountCoin, nil, false)
51+
52+
require.Equal(t, "1", account.ChainID)
53+
require.True(t, strings.EqualFold(
54+
"0x89205a3a3b2a69de6dbf7f01ed13b2108b2c43e7",
55+
account.ContractAddress,
56+
))
57+
require.NotNil(t, account.Decimals)
58+
require.Equal(t, uint(6), *account.Decimals)
59+
}

‎backend/handlers/handlers.go‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,8 @@ type accountJSON struct {
413413
BitsuranceStatus string `json:"bitsuranceStatus"`
414414
CoinName string `json:"coinName"`
415415
ContractAddress string `json:"contractAddress,omitempty"`
416+
ChainID string `json:"chainId,omitempty"`
417+
Decimals *uint `json:"decimals,omitempty"`
416418
ActiveTokens []activeToken `json:"activeTokens,omitempty"`
417419
BlockExplorerTxPrefix string `json:"blockExplorerTxPrefix"`
418420
BlockExplorerAddressPrefix string `json:"blockExplorerAddressPrefix,omitempty"`
@@ -480,15 +482,22 @@ func newAccountJSON(
480482
}
481483

482484
contractAddress := ""
485+
chainID := ""
486+
var decimals *uint
483487
blockExplorerAddressPrefix := ""
484488
if btcCoin, ok := accountCoin.(*btc.Coin); ok {
485489
blockExplorerAddressPrefix = btcCoin.BlockExplorerAddressURLPrefix()
486490
}
487-
if ethCoin, ok := accountCoin.(*eth.Coin); ok && ethCoin.ERC20Token() != nil {
488-
contractAddress = ethCoin.ERC20Token().ContractAddress().Hex()
489-
blockExplorerURLPrefix := ethCoin.BlockExplorerURLPrefix()
490-
if blockExplorerURLPrefix != "" {
491-
blockExplorerAddressPrefix = blockExplorerURLPrefix + "address/"
491+
if ethCoin, ok := accountCoin.(*eth.Coin); ok {
492+
chainID = ethCoin.ChainIDstr()
493+
coinDecimals := ethCoin.Decimals(false)
494+
decimals = &coinDecimals
495+
if ethCoin.ERC20Token() != nil {
496+
contractAddress = ethCoin.ERC20Token().ContractAddress().Hex()
497+
blockExplorerURLPrefix := ethCoin.BlockExplorerURLPrefix()
498+
if blockExplorerURLPrefix != "" {
499+
blockExplorerAddressPrefix = blockExplorerURLPrefix + "address/"
500+
}
492501
}
493502
}
494503

@@ -497,6 +506,8 @@ func newAccountJSON(
497506
BitsuranceStatus: accountConfig.InsuranceStatus,
498507
CoinName: accountCoin.Name(),
499508
ContractAddress: contractAddress,
509+
ChainID: chainID,
510+
Decimals: decimals,
500511
ActiveTokens: activeTokens,
501512
BlockExplorerTxPrefix: accountCoin.BlockExplorerTransactionURLPrefix(),
502513
BlockExplorerAddressPrefix: blockExplorerAddressPrefix,

‎frontends/web/src/api/account.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ export type TAccountBase = {
5353
export type TAccount = TAccountBase & {
5454
coinName: string;
5555
contractAddress?: string;
56+
chainId?: string;
57+
decimals?: number;
5658
activeTokens?: TActiveToken[];
5759
blockExplorerTxPrefix: string;
5860
blockExplorerAddressPrefix?: string;

‎frontends/web/src/locales/en/app.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1854,6 +1854,7 @@
18541854
"invalidAddress": "invalid address",
18551855
"invalidAmount": "invalid amount",
18561856
"invalidData": "invalid data",
1857+
"paymentRequestAccountMismatch": "This payment request is for a different asset. Open the matching account and try again.",
18571858
"syncInProgress": "The account is still syncing. Please wait until syncing is complete and try again."
18581859
},
18591860
"fee": {

‎frontends/web/src/routes/account/send/payment-request.test.ts‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
import { describe, expect, it } from 'vitest';
4+
import type { TAccount } from '@/api/account';
5+
import { parseEthereumPaymentRequest } from './payment-request';
6+
7+
const ethereumAccount: TAccount = {
8+
keystore: {
9+
watchonly: false,
10+
rootFingerprint: 'f23ab988',
11+
name: 'BitBox02',
12+
lastConnected: '',
13+
connected: true,
14+
},
15+
active: true,
16+
coinCode: 'eth',
17+
coinUnit: 'ETH',
18+
coinName: 'Ethereum',
19+
code: 'eth-account',
20+
name: 'Ethereum Account',
21+
isToken: false,
22+
blockExplorerTxPrefix: 'https://example.com/tx/',
23+
chainId: '1',
24+
decimals: 18,
25+
};
26+
27+
const tokenAccount: TAccount = {
28+
...ethereumAccount,
29+
coinCode: 'erc20Test',
30+
coinUnit: 'USDT',
31+
coinName: 'Test token',
32+
code: 'token-account',
33+
name: 'Token Account',
34+
isToken: true,
35+
contractAddress: '0x89205a3a3b2a69de6dbf7f01ed13b2108b2c43e7',
36+
decimals: 6,
37+
};
38+
39+
describe('parseEthereumPaymentRequest', () => {
40+
it.each([
41+
[
42+
'ethereum:0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359',
43+
ethereumAccount,
44+
{
45+
success: true,
46+
recipient: '0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359',
47+
},
48+
],
49+
[
50+
'ethereum:0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359',
51+
tokenAccount,
52+
{
53+
success: true,
54+
recipient: '0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359',
55+
},
56+
],
57+
[
58+
'ethereum:pay-0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359@01?value=2.014e18',
59+
ethereumAccount,
60+
{
61+
success: true,
62+
recipient: '0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359',
63+
amount: '2.014',
64+
},
65+
],
66+
[
67+
'ethereum:0x89205A3A3B2A69DE6DBF7F01ED13B2108B2C43E7@1/transfer'
68+
+ '?address=0x8e23ee67d1332ad560396262c48ffbb01f93d052&uint256=1.2e6',
69+
tokenAccount,
70+
{
71+
success: true,
72+
recipient: '0x8e23ee67d1332ad560396262c48ffbb01f93d052',
73+
amount: '1.2',
74+
},
75+
],
76+
[
77+
'ethereum:0x89205a3a3b2a69de6dbf7f01ed13b2108b2c43e7/transfer'
78+
+ '?address=0x8e23ee67d1332ad560396262c48ffbb01f93d052',
79+
tokenAccount,
80+
{
81+
success: true,
82+
recipient: '0x8e23ee67d1332ad560396262c48ffbb01f93d052',
83+
},
84+
],
85+
])('parses %s', (uri, account, expected) => {
86+
expect(parseEthereumPaymentRequest(uri, account)).toEqual(expected);
87+
});
88+
89+
it.each([
90+
['https://example.com/payment', ethereumAccount],
91+
['ethereum:0x1234?value=1', ethereumAccount],
92+
['ethereum:0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359@11155111?value=1e18', ethereumAccount],
93+
['ethereum:0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359?value=1.2', ethereumAccount],
94+
['ethereum:0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359?gas=21000', ethereumAccount],
95+
['ethereum:0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359?value=1&value=2', ethereumAccount],
96+
['ethereum:0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359/approve', ethereumAccount],
97+
['ethereum:0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359?value=1#fragment', ethereumAccount],
98+
])('rejects %s', (uri, account) => {
99+
expect(parseEthereumPaymentRequest(uri, account)).toEqual({ success: false });
100+
});
101+
102+
it.each([
103+
[
104+
'ethereum:0x0000000000000000000000000000000000000001/transfer'
105+
+ '?address=0x8e23ee67d1332ad560396262c48ffbb01f93d052&uint256=1',
106+
tokenAccount,
107+
],
108+
[
109+
'ethereum:0x89205a3a3b2a69de6dbf7f01ed13b2108b2c43e7/transfer'
110+
+ '?address=0x8e23ee67d1332ad560396262c48ffbb01f93d052&uint256=1',
111+
ethereumAccount,
112+
],
113+
[
114+
'ethereum:0xfb6916095ca1df60bb79Ce92ce3ea74c37c5d359?value=1e18',
115+
tokenAccount,
116+
],
117+
])('reports an account mismatch for %s', (uri, account) => {
118+
expect(parseEthereumPaymentRequest(uri, account)).toEqual({
119+
success: false,
120+
error: 'accountMismatch',
121+
});
122+
});
123+
});

0 commit comments

Comments
 (0)