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.

Sign up for GitHub

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

txnbuild: add AddSignatureDecorated to FeeBumpTransaction #4174

Merged
merged 3 commits into from
Jan 12, 2022
Merged

txnbuild: add AddSignatureDecorated to FeeBumpTransaction #4174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eacd075
txnbuild: add AddSignatureDecorate to FeeBumpTransaction
leighmcculloch Jan 11, 2022
26a8a23
Merge branch 'master' into txbuild-adddecoratesignature-feebump
leighmcculloch Jan 11, 2022
828492e
Merge branch 'master' into txbuild-adddecoratesignature-feebump
leighmcculloch Jan 12, 2022
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
11 changes: 11 additions & 0 deletions txnbuild/transaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,17 @@ func (t *FeeBumpTransaction) ClearSignatures() (*FeeBumpTransaction, error) {
return t.clone(nil), nil
}

// AddSignatureDecorated returns a new FeeBumpTransaction instance which extends the current instance
// with an additional decorated signature(s).
func (t *FeeBumpTransaction) AddSignatureDecorated(signature ...xdr.DecoratedSignature) (*FeeBumpTransaction, error) {
extendedSignatures, err := concatSignatureDecorated(t.envelope, t.Signatures(), signature)
if err != nil {
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

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

generally prefer wrapping errors so the context isn't lost

Copy link
Member Author

@leighmcculloch leighmcculloch Jan 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I often do that too, but there is a series of add signature functions of which this is just one variation that was missing on this type and this is how they all work, with the errors being well defined within the concat functions.

}

return t.clone(extendedSignatures), nil
}

// AddSignatureBase64 returns a new FeeBumpTransaction instance which extends the current instance
// with an additional signature derived from the given base64-encoded signature.
func (t *FeeBumpTransaction) AddSignatureBase64(network, publicKey, signature string) (*FeeBumpTransaction, error) {
Expand Down
45 changes: 45 additions & 0 deletions txnbuild/transaction_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1638,6 +1638,51 @@ func TestAddSignatureDecorated(t *testing.T) {
}
}

func TestFeeBumpTransaction_AddSignatureDecorated(t *testing.T) {
kp0 := newKeypair0()
kp1 := newKeypair1()

tx, err := NewTransaction(TransactionParams{
Copy link
Contributor

Choose a reason for hiding this comment

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

this new transaction creation could go into a helper function since it seems to be used as the standard transaction across multiple tests

Copy link
Member Author

Choose a reason for hiding this comment

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

It could, but then the tests would become more coupled together. I've found it is easier to maintain tests in the long run if they are mostly independent of each other. Helper functions can be helpful, but they will also narrow the inputs to tests, or balloon into big configurable things of their own. Also this way they act as an example of the public API as much as possible.

SourceAccount: &SimpleAccount{kp0.Address(), int64(9605939170639897)},
Operations: []Operation{&CreateAccount{
Destination: "GCCOBXW2XQNUSL467IEILE6MMCNRR66SSVL4YQADUNYYNUVREF3FIV2Z",
Amount: "10",
SourceAccount: kp1.Address(),
}},
BaseFee: MinBaseFee,
Timebounds: NewInfiniteTimeout(),
})
require.NoError(t, err)
tx, err = tx.Sign(network.TestNetworkPassphrase, kp0, kp1)
require.NoError(t, err)

fbtx, err := NewFeeBumpTransaction(FeeBumpTransactionParams{
Inner: tx,
FeeAccount: kp0.Address(),
BaseFee: MinBaseFee,
})
require.NoError(t, err)
require.Len(t, fbtx.Signatures(), 0)

fbtxHash, err := fbtx.Hash(network.TestNetworkPassphrase)
require.NoError(t, err)

sig, err := kp0.SignDecorated(fbtxHash[:])
require.NoError(t, err)
fbtxWithSig, err := fbtx.AddSignatureDecorated(sig)
require.NoError(t, err)
require.Len(t, fbtx.Signatures(), 0)
require.Len(t, fbtxWithSig.Signatures(), 1)

sig, err = kp1.SignDecorated(fbtxHash[:])
require.NoError(t, err)
fbtxWithTwoSigs, err := fbtxWithSig.AddSignatureDecorated(sig)
require.NoError(t, err)
require.Len(t, fbtx.Signatures(), 0)
require.Len(t, fbtxWithSig.Signatures(), 1)
require.Len(t, fbtxWithTwoSigs.Signatures(), 2)
}

func TestAddSignatureBase64(t *testing.T) {
kp0 := newKeypair0()
kp1 := newKeypair1()
Expand Down