stacks-archive / stacks-transactions-js

The JavaScript library for generating Stacks 2.0 transactions
19 stars 17 forks source link

Array buffer alternative implementation #113

Closed hozzjss closed 3 years ago

hozzjss commented 3 years ago

Use the following template to create your pull request

Description

As a developer I should consume the BufferArray class to manage an array of buffers with extra functionality

  1. Typescript gives out an error when extending native classes as it cannot convert these class to ES5 relevant constructor functions
  2. Array buffer only uses push from the Array native class so just implemented relevant fns instead, updating the class to fulfill all of Array's prototype functions is futile as they evolve over time, and extending the Array class is an overkill looking at the uses of this class
  3. Intellisense will not be as confusing or glittery anymore with the array prototype functions
  4. Relevant issue https://github.com/blockstack/stacks-transactions-js/issues/111

Example from existing code:

export function serializePrincipal(principal: PostConditionPrincipal): Buffer {
  const bufferArray: BufferArray = new BufferArray();
  bufferArray.push(Buffer.from([principal.prefix]));
  bufferArray.push(serializeAddress(principal.address));
  if (principal.prefix === PostConditionPrincipalID.Contract) {
    bufferArray.push(serializeLPString(principal.contractName));
  }
  return bufferArray.concatBuffer();
}

For details refer to issue #123

Type of Change

Does this introduce a breaking change?

No

Are documentation updates required?

No, the class's functionality is limited

Testing information

No testing needed Provide context on how tests should be performed.

  1. Is testing required for this change? No.
  2. If it’s a bug fix, list steps to reproduce the bug Build the lib with target lower than ES6
  3. Briefly mention affected code paths src/utils.ts
  4. List other affected projects if possible None
  5. Things to watch out for when testing Nothing

    Checklist

    • [x] Code is commented where needed
    • [ ] Unit test coverage for new or modified code paths
    • [x] npm run test passes
    • [x] Changelog is updated
    • [x] Tag 1 of @yknl for review
CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

hozzjss commented 3 years ago

This is my first PR ever, glad it went okay :grin: