Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update address to follow specs #552

Merged
merged 4 commits into from
Dec 18, 2024

Conversation

danielailie
Copy link
Contributor

No description provided.

@danielailie danielailie self-assigned this Dec 17, 2024
Base automatically changed from TOOL-405-update-to-use-new-methods-on-token-transfer to feat/next December 17, 2024 14:11
andreibancioiu
andreibancioiu previously approved these changes Dec 17, 2024
@@ -65,7 +65,7 @@ describe("test smart contract interactor", function () {
const hexBar = "4241522d356263303866";
const hexLKMEX = "4c4b4d45582d616162393130";
const hexNFT = "4d4f532d623962346232";
const hexContractAddress = new Address(contract.getAddress().toBech32()).hex();
const hexContractAddress = new Address(contract.getAddress().toBech32()).toHex();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could have been:

const hexContractAddress = contract.getAddress().toHex();

@@ -288,7 +288,7 @@ export class SmartContract implements ISmartContract {
* @param nonce The owner nonce used for the deployment transaction
*/
static computeAddress(owner: Address, nonce: bigint): Address {
const deployer = Address.fromBech32(owner.toBech32());
const deployer = Address.newFromBech32(owner.toBech32());
Copy link
Contributor

Choose a reason for hiding this comment

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

owner was already of type Address.

@@ -63,7 +63,7 @@ export class AccountTransactionsFactory {
createTransactionForSettingGuardian(sender: Address, options: SetGuardianInput): Transaction {
const dataParts = [
"SetGuardian",
Address.fromBech32(options.guardianAddress.bech32()).toHex(),
Address.newFromBech32(options.guardianAddress.toBech32()).toHex(),
Copy link
Contributor

Choose a reason for hiding this comment

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

could have been:

options.guardianAddress.toHex()

@@ -185,7 +185,7 @@ export class SmartContractTransactionsFactory {
contract: Address;
newOwner: Address;
}): Transaction {
const dataParts = ["ChangeOwnerAddress", Address.fromBech32(options.newOwner.toBech32()).toHex()];
const dataParts = ["ChangeOwnerAddress", Address.newFromBech32(options.newOwner.toBech32()).toHex()];
Copy link
Contributor

Choose a reason for hiding this comment

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

could have been:

options.newOwner.toHex()

Comment on lines 56 to 58
export function addressToHex(address: Address): string {
const buffer = Address.fromBech32(address.toString()).pubkey();
const buffer = Address.newFromBech32(address.toString()).getPublicKey();
return buffer.toString("hex");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is not needed since an Address can be eaasily converted to hex string. If still needed perhaps simply return address.toHex()?

const addressComputer = new AddressComputer();
return addressComputer.computeContractAddress(deployer, BigInt(nonce.valueOf()));
return addressComputer.computeContractAddress(owner, BigInt(nonce.valueOf()));
Copy link
Contributor

Choose a reason for hiding this comment

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

The nonce is already of type bigint.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can be chaged in a future PR.

@danielailie danielailie merged commit 483084f into feat/next Dec 18, 2024
4 checks passed
@danielailie danielailie deleted the TOOL-407-update-address-to-follow-specs branch December 18, 2024 09:29
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.

3 participants