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

Implement getTransactionId for transfers on the Predicate class #1451

Closed
arboleya opened this issue Nov 25, 2023 · 3 comments · Fixed by #1485
Closed

Implement getTransactionId for transfers on the Predicate class #1451

arboleya opened this issue Nov 25, 2023 · 3 comments · Fixed by #1485
Assignees
Labels
feat Issue is a feature

Comments

@arboleya
Copy link
Member

arboleya commented Nov 25, 2023

Given this example:

// Prepare the transaction.
const tx = await state.predicate
    .transfer(recipient, value, types.BaseAssetId, {
        gasPrice: 1,
        gasLimit: 3_500_000,
});

// Wait for result.
const result = await tx.waitForResult();

One might need to get the transaction ID before sending the transaction:

// Getting chain Id
const chainId = provider.getChainId();

// Get tx ID
const txId = hashTransaction(txRequest, chainId); // <— like this

// Sending the tx to the chain
const tx = await myWallet.sendTransaction(txRequest);

// Waiting for the tx to be processed
const res = await tx.waitForResult();

The problem is that the transfer method creates a ScriptTransactionRequest, funds it, and sends in the same call.

To intercept the request and compute the transaction ID before it's sent, we must fragment this chain-of-action and expose the request instance in isolation via a method utility or something similar, enabling one to modify it before submitting the transaction and even getTransactionId() before calling submit().

From this:

predicate.transfer(...);

To something like this:

// first iteration
const txID = predicate.getTransactionId();
predicate.transfer(...);

And then this:

// second iteration
const transfer = predicate.transfer(...);
const txID = transfer.getTransactionId()

transfer.submit(...);

Warning

Remember to roll this update in two phases:

  1. Add method getTransferTransactionId() and keep transfer() as is, but with a deprecation notice

    • No breaking changes here; annotate deprecations.
  2. Implement .transfer().getTransactionId().submit()

    • Possible breaking changes; may apply deprecations.
@arboleya arboleya added the feat Issue is a feature label Nov 25, 2023
@arboleya arboleya changed the title Add ability to get transaction ID for a transfer before sending it Implement getTransactionId for transfers Nov 25, 2023
@arboleya arboleya changed the title Implement getTransactionId for transfers Implement getTransactionId for transfers on the Predicate class Nov 27, 2023
@arboleya
Copy link
Member Author

Although adding this getTransactionId method will be handy, we should improve the Predicate DX afterward via:

If Predicate had the same DX as Contract, we could do something like this:

const transaction = contract.functions
  .transfer(amountToTransfer, BaseAssetId, someAddress)
  .callParams({
    forward: [amountToForward, BaseAssetId],
  })
  .txParams({
    gasPrice: 1,
    gasLimit: 1,
    variableOutputs: 1,
  });const request = await transaction.getTransactionRequest();// before sending
console.log('gasLimit: ', request.gasLimit);
console.log('gasPrice: ', request.gasPrice);
console.log('Tx ID: ', hashTransaction(request, 0));// sending
const { gasUsed } = await transaction.call();// after sending
console.log({ gasUsed });

@Dhaiwat10 Dhaiwat10 self-assigned this Nov 27, 2023
@arboleya
Copy link
Member Author

Either way, instead of getting the TX ID like this:

const transaction = contract.functions.xyz();
const request = await transaction.getTransactionRequest();
const txID = hashTransaction(request, 0);

We could have something more straightforward:

const transaction = contract.functions.xyz();
const txID = transaction.getTransactionId();

This would eliminate the need to use hashTransaction manually.

This rationale applies to both Contract and Predicate classes.

@arboleya
Copy link
Member Author

arboleya commented Nov 27, 2023

No required but related:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Issue is a feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants