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
42 changes: 40 additions & 2 deletions modules/sdk-coin-algo/src/algo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
TransactionExplanation,
TransactionRecipient,
TransactionType,
InvalidTransactionError,
UnexpectedAddressError,
VerifyAddressOptions,
VerifyTransactionOptions,
Expand Down Expand Up @@ -74,6 +75,7 @@ export interface VerifyAlgoAddressOptions extends VerifyAddressOptions {
export interface AlgoTransactionExplanation extends TransactionExplanation {
memo?: string;
type?: string | number;
reKeyTo?: string;
voteKey?: string;
selectionKey?: string;
voteFirst?: number;
Expand All @@ -91,6 +93,13 @@ export interface TransactionOperation {
export interface SignTransactionOptions extends BaseSignTransactionOptions {
txPrebuild: TransactionPrebuild;
prv: string;
/**
* Explicit opt-in required to sign a transaction that sets `reKeyTo`: the exact rekey
* address that was shown to and approved by the operator. A rekey hands spending
* authority of the account to another address, so signing throws if the decoded
* transaction rekeys to any other address.
*/
allowReKeyTo?: string;
}

export interface TransactionPrebuild {
Expand Down Expand Up @@ -357,7 +366,7 @@ export class Algo extends BaseCoin {
* @param params
*/
async explainTransaction(params: ExplainTransactionOptions): Promise<AlgoTransactionExplanation | undefined> {
const txHex = params.txHex || (params.halfSigned && params.halfSigned.txHex);
const txHex = (params.halfSigned && params.halfSigned.txHex) || params.txHex;
if (!txHex || !params.feeInfo) {
throw new Error('missing explain tx parameters');
}
Expand Down Expand Up @@ -412,6 +421,13 @@ export class Algo extends BaseCoin {
operations,
};

// A rekey silently reassigns spending authority of the account, so it must never
// be hidden from the explanation shown for approval.
if (txJson.reKeyTo) {
displayOrder.push('reKeyTo');
explanationResult.reKeyTo = txJson.reKeyTo;
}
Comment thread
mukeshsp marked this conversation as resolved.
Resolved

if (txJson.tokenId) {
explanationResult.tokenId = txJson.tokenId;
}
Expand All @@ -432,7 +448,7 @@ export class Algo extends BaseCoin {
'voteKeyDilution',
];

return {
const explanationResult: AlgoTransactionExplanation = {
displayOrder,
id: txJson.id,
outputAmount: '0',
Expand All @@ -448,6 +464,14 @@ export class Algo extends BaseCoin {
voteLast: txJson.voteLast,
voteKeyDilution: txJson.voteKeyDilution,
};

// A keyreg transaction can also carry a rekey; surface it like in the send case.
if (txJson.reKeyTo) {
displayOrder.push('reKeyTo');
explanationResult.reKeyTo = txJson.reKeyTo;
}
Comment thread
claude[bot] marked this conversation as resolved.
Resolved

return explanationResult;
}
}

Expand Down Expand Up @@ -550,6 +574,20 @@ export class Algo extends BaseCoin {
*/
async signTransaction(params: SignTransactionOptions): Promise<SignedTransaction> {
const { txHex, signers, prv, isHalfSigned, numberSigners } = this.verifySignTransactionParams(params);
// A rekey reassigns spending authority of the account; refuse to sign it unless the
// caller passes the exact rekey address the operator approved. A bare opt-in would
// also approve a different rekey target hidden in the bytes being signed.
// Note: this guards Algo.signTransaction only; other signing entry points (e.g.
// callers driving TransactionBuilder.sign directly) are not covered.
const { txn } = Utils.decodeAlgoTxn(txHex);
if (txn.reKeyTo) {
const reKeyAddress = algosdk.encodeAddress(txn.reKeyTo.publicKey);
if (params.allowReKeyTo !== reKeyAddress) {
throw new InvalidTransactionError(
`transaction rekeys the account to ${reKeyAddress}: refusing to sign unless the rekey is confirmed intentional (pass allowReKeyTo with the approved rekey address)`
);
}
}
const factory = this.getBuilder();
const txBuilder = factory.from(txHex);
txBuilder.numberOfRequiredSigners(numberSigners);
Expand Down
1 change: 1 addition & 0 deletions modules/sdk-coin-algo/src/lib/ifaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface TxData {
genesisID: string;
genesisHash: string | Buffer;
closeRemainderTo?: string;
reKeyTo?: string;
nonParticipation?: boolean;
txType?: string;
tokenName?: string;
Expand Down
5 changes: 3 additions & 2 deletions modules/sdk-coin-algo/src/lib/keyRegistrationBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ export class KeyRegistrationBuilder extends TransactionBuilder {
undefined, // voteFirst param
undefined, // voteLast param
undefined, // voteKeyDilution param
this.suggestedParams
this.suggestedParams,
this._reKeyTo
);
}

Expand All @@ -157,7 +158,7 @@ export class KeyRegistrationBuilder extends TransactionBuilder {
this._voteLast,
this._voteKeyDilution,
this.suggestedParams,
undefined, // reKeyTo param
this._reKeyTo,
undefined, // nonParticipation param
this._stateProofKey
);
Expand Down
3 changes: 3 additions & 0 deletions modules/sdk-coin-algo/src/lib/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,9 @@ export class Transaction extends BaseTransaction {
if (this._algoTransaction.closeRemainderTo) {
result.closeRemainderTo = algosdk.encodeAddress(this._algoTransaction.closeRemainderTo.publicKey);
}
if (this._algoTransaction.reKeyTo) {
result.reKeyTo = algosdk.encodeAddress(this._algoTransaction.reKeyTo.publicKey);
}
if (this.type === TransactionType.Send) {
result.to = algosdk.encodeAddress(this._algoTransaction.to.publicKey);
result.amount = this._algoTransaction.amount.toString();
Expand Down
171 changes: 171 additions & 0 deletions modules/sdk-coin-algo/test/unit/algo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,52 @@ describe('ALGO:', function () {
explain.outputs[0].amount.should.equal(amount.toString());
explain.outputs[0].address.should.equal(destination);
});

it('should surface the rekey target when explaining a transfer transaction that rekeys the account', async function () {
const sender = AlgoResources.accounts.account1.address;
const destination = AlgoResources.accounts.account2.address;
const rekeyAddress = AlgoResources.accounts.account3.address;
const txBuilder = buildBaseTransferTransaction({ sender, destination, amount: 10000 });
txBuilder.reKeyTo({ address: rekeyAddress });
const unsignedTransaction = await txBuilder.build();
const unsignedHex = Buffer.from(unsignedTransaction.toBroadcastFormat()).toString('hex');
const explain = await basecoin.explainTransaction({
txHex: unsignedHex,
feeInfo: { fee: '1000' },
});
explain.reKeyTo.should.equal(rekeyAddress);
explain.displayOrder.should.containEql('reKeyTo');
explain.outputs[0].address.should.equal(destination);
});

it('should explain the halfSigned transaction when both txHex and halfSigned txHex are provided', async function () {
const sender = AlgoResources.accounts.account1.address;
const destination = AlgoResources.accounts.account2.address;
const rekeyAddress = AlgoResources.accounts.account3.address;

// plain unsigned payment without a rekey, used as the misleading txHex
const plainHex = Buffer.from(
(await buildBaseTransferTransaction({ sender, destination, amount: 10000 }).build()).toBroadcastFormat()
).toString('hex');

// half-signed payment that rekeys the account, used as the halfSigned txHex
const rekeyedBuilder = buildBaseTransferTransaction({ sender, destination, amount: 10000 });
rekeyedBuilder.reKeyTo({ address: rekeyAddress });
rekeyedBuilder.numberOfSigners(2);
rekeyedBuilder.setSigners([AlgoResources.accounts.account1.address, AlgoResources.accounts.account3.address]);
rekeyedBuilder.sign({ key: AlgoResources.accounts.account1.prvKey });
const halfSignedHex = Buffer.from((await rekeyedBuilder.build()).toBroadcastFormat()).toString('hex');

// signTransaction signs the halfSigned hex when both are present, so the
// explanation must come from the same bytes, not from txHex
const explain = await basecoin.explainTransaction({
txHex: plainHex,
halfSigned: { txHex: halfSignedHex },
feeInfo: { fee: '1000' },
});
explain.reKeyTo.should.equal(rekeyAddress);
explain.displayOrder.should.containEql('reKeyTo');
});
});

describe('Asset Transfer Builder: ', () => {
Expand Down Expand Up @@ -349,6 +395,23 @@ describe('ALGO:', function () {
explain.outputs[0].address.should.equal(destination);
explain.tokenId.should.equal(1);
});

it('should surface the rekey target when explaining an asset transfer transaction that rekeys the account', async function () {
const sender = AlgoResources.accounts.account1.address;
const destination = AlgoResources.accounts.account2.address;
const rekeyAddress = AlgoResources.accounts.account3.address;
const txBuilder = buildBaseAssetTransferTransaction({ sender, destination, amount: 10000, tokenId: 1 });
txBuilder.reKeyTo({ address: rekeyAddress });
const unsignedTransaction = await txBuilder.build();
const unsignedHex = Buffer.from(unsignedTransaction.toBroadcastFormat()).toString('hex');
const explain = await basecoin.explainTransaction({
txHex: unsignedHex,
feeInfo: { fee: '1000' },
});
explain.reKeyTo.should.equal(rekeyAddress);
explain.displayOrder.should.containEql('reKeyTo');
explain.outputs[0].address.should.equal(destination);
});
});

describe('Wallet Init Builder: ', () => {
Expand Down Expand Up @@ -538,6 +601,21 @@ describe('ALGO:', function () {
explain.voteLast.should.equal(100);
explain.voteKeyDilution.should.equal(9);
});

it('should surface the rekey target when explaining a keyreg transaction that rekeys the account', async function () {
const sender = AlgoResources.accounts.account1;
const rekeyAddress = AlgoResources.accounts.account3.address;
const txBuilder = buildBaseKeyRegTransaction({ sender });
txBuilder.reKeyTo({ address: rekeyAddress });
const unsignedTransaction = await txBuilder.build();
const unsignedHex = Buffer.from(unsignedTransaction.toBroadcastFormat()).toString('hex');
const explain = await basecoin.explainTransaction({
txHex: unsignedHex,
feeInfo: { fee: '1000' },
});
explain.reKeyTo.should.equal(rekeyAddress);
explain.displayOrder.should.containEql('reKeyTo');
});
});
describe('Sign transaction', () => {
it('should sign transaction', async function () {
Expand Down Expand Up @@ -648,6 +726,99 @@ describe('ALGO:', function () {
});
signed.txHex.should.equal(AlgoResources.rawTx.transfer.multisig);
});

it('should refuse to sign a transaction that rekeys the account', async function () {
const factory = new TransactionBuilderFactory(coins.get('algo'));
const txBuilder = factory
.getTransferBuilder()
.sender({ address: AlgoResources.accounts.account1.address })
.to({ address: AlgoResources.accounts.account2.address })
.amount(10000)
.isFlatFee(true)
.fee({ fee: '1000' })
.firstRound(1)
.lastRound(100)
.testnet();
txBuilder.reKeyTo({ address: AlgoResources.accounts.account3.address });
const unsignedTx = await txBuilder.build();
const unsignedHex = Buffer.from(unsignedTx.toBroadcastFormat()).toString('hex');

await basecoin
.signTransaction({
txPrebuild: {
txHex: unsignedHex,
keys: [AlgoResources.accounts.account1.pubKey.toString('hex')],
addressVersion: 1,
},
prv: AlgoResources.accounts.account1.prvKey,
})
.should.be.rejectedWith(
new RegExp(
`transaction rekeys the account to ${AlgoResources.accounts.account3.address}: refusing to sign unless the rekey is confirmed intentional`
)
);
});

it('should sign a transaction that rekeys the account when the rekey is explicitly allowed', async function () {
const factory = new TransactionBuilderFactory(coins.get('algo'));
const txBuilder = factory
.getTransferBuilder()
.sender({ address: AlgoResources.accounts.account1.address })
.to({ address: AlgoResources.accounts.account2.address })
.amount(10000)
.isFlatFee(true)
.fee({ fee: '1000' })
.firstRound(1)
.lastRound(100)
.testnet();
txBuilder.reKeyTo({ address: AlgoResources.accounts.account3.address });
const unsignedTx = await txBuilder.build();
const unsignedHex = Buffer.from(unsignedTx.toBroadcastFormat()).toString('hex');

const signed = await basecoin.signTransaction({
txPrebuild: {
txHex: unsignedHex,
keys: [AlgoResources.accounts.account1.pubKey.toString('hex')],
addressVersion: 1,
},
prv: AlgoResources.accounts.account1.prvKey,
allowReKeyTo: AlgoResources.accounts.account3.address,
});
assert.strictEqual(typeof signed.txHex, 'string');
});

it('should refuse to sign a rekeyed transaction when the approved rekey address does not match', async function () {
const factory = new TransactionBuilderFactory(coins.get('algo'));
const txBuilder = factory
.getTransferBuilder()
.sender({ address: AlgoResources.accounts.account1.address })
.to({ address: AlgoResources.accounts.account2.address })
.amount(10000)
.isFlatFee(true)
.fee({ fee: '1000' })
.firstRound(1)
.lastRound(100)
.testnet();
txBuilder.reKeyTo({ address: AlgoResources.accounts.account3.address });
const unsignedTx = await txBuilder.build();
const unsignedHex = Buffer.from(unsignedTx.toBroadcastFormat()).toString('hex');

await basecoin
.signTransaction({
txPrebuild: {
txHex: unsignedHex,
keys: [AlgoResources.accounts.account1.pubKey.toString('hex')],
addressVersion: 1,
},
prv: AlgoResources.accounts.account1.prvKey,
allowReKeyTo: AlgoResources.accounts.account2.address,
})
.should.be.rejectedWith(
new RegExp(
`transaction rekeys the account to ${AlgoResources.accounts.account3.address}: refusing to sign unless the rekey is confirmed intentional`
)
);
});
});

describe('Sign message', () => {
Expand Down
Loading