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

Add support for partially signed tx, closes #38 #42

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

safanaj
Copy link

@safanaj safanaj commented Nov 15, 2022

  • allow to set additional witnesses for partially signed tx preparation
  • change calculateMinFee to consider additional witnesses that will increase the tx length. It is still not clear how to precisely calculate the bytes for witnesses, in this implementation for each additional witnesses there will be an additional 100 bytes, (32 public key, 64 signature, 4 index/key in cbor). Those are not actually enough so we are considering 1 additional byte for each additional witness after the first.

* allow to set additional witnesses for partially signed tx preparation
* change calculateMinFee to consider additional witnesses that will
  increase the tx length. It is still not clear how to precisely
  calculate the bytes for witnesses, in this implementation
  for each additional witnesses there will be an additional 100 bytes,
  (32 public key, 64 signature, 4 index/key in cbor). Those are not
  actually enough so we are considering 1 additional byte
  for each additional witness after the first.
@safanaj
Copy link
Author

safanaj commented Nov 15, 2022

ping @echovl

tx_builder.go Outdated
@@ -143,6 +151,14 @@ func (tb *TxBuilder) MinCoinsForTxOut(txOut *TxOutput) Coin {
func (tb *TxBuilder) calculateMinFee() Coin {
txBytes := tb.tx.Bytes()
txLength := uint64(len(txBytes))
// for each additional witnesses there will be an additional 100 bytes,
// (32 public key, 64 signature, 4 index/key in cbor)
txLength += uint64(tb.additionalWitnesses * 100)
Copy link
Owner

Choose a reason for hiding this comment

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

instead of doing this, we could just encode the tx using placeholder witnesses keys so we can have an accurate tx length

Copy link
Owner

Choose a reason for hiding this comment

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

what do you think? @safanaj

Copy link
Author

Choose a reason for hiding this comment

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

hey @echovl do you mean adding some "fake" xprvkey with .Sign ? not sure how to do that and/or to get your idea.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @echovl I think I got your point, now in the calculateMinFee the additional witnesses are temporary added just to compute the length, the size of VKey and Signature are literal, those could be get from github.com/echovl/ed25519 but I didn't add that import just for 32 and 64 literals.

Signed-off-by: Marco Bardelli <bardelli.marco@gmail.com>
Signed-off-by: Marco Bardelli <bardelli.marco@gmail.com>
@safanaj
Copy link
Author

safanaj commented Jan 14, 2023

ping @echovl

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.

2 participants